[Xcb] bug report

[Sidney Cadot]

Hi all,

Is this the appropriate place for bug reports on the xcb codebase? If
so, see below for a bug in xcb_get_auth_info().

It is not entirely clear to me what the function is trying to
accomplish (I feel the code is rather unclear, to be honest), so I
cannot propose a patch. Nonetheless, I would appreciate it is this bug
were fixed as it continuously shows up when valgrind'ing my programs.

Best regards, SIdney


In src/xcb_auth.c the function "_xcb_get_auth_info" is defined as follows:

   289  int _xcb_get_auth_info(int fd, xcb_auth_info_t *info, int display)
   290  {
   291      /* code adapted from Xlib/ConnDis.c, xtrans/Xtranssocket.c,
   292         xtrans/Xtransutils.c */
   293      struct sockaddr *sockname = NULL;
   294      int gotsockname = 0;
   295      Xauth *authptr = 0;
   296      int ret = 1;
   297
   298      /* Some systems like hpux or Hurd do not expose peer names
   299       * for UNIX Domain Sockets, but this is irrelevant,
   300       * since compute_auth() ignores the peer name in this
   301       * case anyway.*/
   302      if ((sockname = get_peer_sock_name(getpeername, fd)) == NULL)
   303      {
   304          if ((sockname = get_peer_sock_name(getsockname, fd)) == NULL)
   305              return 0;   /* can only authenticate sockets */
   306          if (sockname->sa_family != AF_UNIX)
   307          {
   308              free(sockname);
   309              return 0;   /* except for AF_UNIX, sockets should
have peernames */
   310          }
   311          gotsockname = 1;
   312      }
   313
   314      authptr = get_authptr(sockname, display);
   315      if (authptr == 0)
   316      {
   317          free(sockname);
   318          return 0;   /* cannot find good auth data */
   319      }
   320
   321      info->namelen = memdup(&info->name, authptr->name,
authptr->name_length);
   322      if (!info->namelen)
   323          goto no_auth;   /* out of memory */
   324
   325      if (!gotsockname && (sockname =
get_peer_sock_name(getsockname, fd)) == NULL)
   326      {
   327          free(info->name);
   328          goto no_auth;   /* can only authenticate sockets */
   329      }
   330
   331      ret = compute_auth(info, authptr, sockname);
   332      if(!ret)
   333      {
   334          free(info->name);
   335          goto no_auth;   /* cannot build auth record */
   336      }
   337
   338      free(sockname);
   339      sockname = NULL;
   340
   341      XauDisposeAuth(authptr);
   342      return ret;
   343
   344   no_auth:
   345      free(sockname);
   346
   347      info->name = 0;
   348      info->namelen = 0;
   349      XauDisposeAuth(authptr);
   350      return 0;
   351  }

This code has a memory leak:

(1) In line 302, get_peer_sock_name() is called. Supposing it returns
a non-NULL value, it is a malloc'ed string that must be freed. Note
that if this is the case, the variable 'gotsockname' is /not/ set on
line 311. It remains at zero.

(2) Proceeding to line 325: since 'gotsockname' is now zero, another
call to get_peer_sock_name() is executed. The value of 'sockname' is
overwritten with a new malloced string. Since the previous value was
not free()d at this point, we have a memory leak.