Bug 104123 - ComputeLocalClient crashes if the client cmd name begins with a colon (:)
Summary: ComputeLocalClient crashes if the client cmd name begins with a colon (:)
Alias: None
Product: xorg
Classification: Unclassified
Component: Server/General (show other bugs)
Version: git
Hardware: All Linux (All)
: medium major
Assignee: Xorg Project Team
QA Contact: Xorg Project Team
Depends on:
Reported: 2017-12-05 16:17 UTC by kailoran+fd
Modified: 2018-12-13 22:38 UTC (History)
0 users

See Also:
i915 platform:
i915 features:

proposed fix (689 bytes, text/plain)
2017-12-05 16:17 UTC, kailoran+fd
no flags Details

Description kailoran+fd 2017-12-05 16:17:44 UTC
Created attachment 135983 [details]
proposed fix

ComputeLocalClient in os/access.c is buggy, it incorrectly frees a pointer returned by strtok(cmd, ":") assuming it will still point to cmd. It does not if cmd starts with a ":" -- it's advanced to the first non-colon character, causing an invalid-free crash.

This issue can be reproduced by having a program whose name begins with a colon get to the client connection code. For example, by symlinking /usr/bin/vim to ~/bin/:e and executing :e with ~/bin in PATH. This crashes Xorg completely. Some programs cause the crash, some don't, probably depending on whether they try to use xorg. For me, vim, emacs and gedit all crash when called via a symlink beginning with ":".

I had some issues getting a backtrace on my Ubuntu 16.04, but reproduced the issue on a Debian 9 on a different machine:

#0  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#1  0x00007fd24612719a in __GI_abort () at abort.c:89
#2  0x000055e7136ec042 in OsAbort () at ../../../../os/utils.c:1361
#3  0x000055e7136f6098 in AbortServer () at ../../../../os/log.c:877
#4  0x000055e7136f65b8 in FatalError (f=0x55e71371d5c0 "Caught signal %d (%s). Server aborting\n") at ../../../../os/log.c:1015
#5  0x000055e7136e8168 in OsSigHandler (signo=6, sip=0x7ffd018da570, unused=0x7ffd018da440) at ../../../../os/osinit.c:154
#6  <signal handler called>
#7  __GI_raise (sig=sig@entry=6) at ../sysdeps/unix/sysv/linux/raise.c:51
#8  0x00007fd24612719a in __GI_abort () at abort.c:89
#9  0x00007fd246164310 in __libc_message (do_abort=do_abort@entry=2, fmt=fmt@entry=0x7fd24625eb10 "*** Error in `%s': %s: 0x%s ***\n") at ../sysdeps/posix/libc_fatal.c:175
#10 0x00007fd24616a96e in malloc_printerr (action=3, str=0x7fd24625b667 "free(): invalid pointer", ptr=<optimized out>, ar_ptr=<optimized out>) at malloc.c:5079
#11 0x00007fd24616b1ce in _int_free (av=0x7fd24648fb00 <main_arena>, p=0x55e715b5c261, have_lock=0) at malloc.c:3875
#12 0x000055e7136e0f15 in ComputeLocalClient (client=0x55e715ae14b0) at ../../../../os/access.c:1148
#13 0x000055e7136e448e in AllocNewConnection (trans_conn=0x55e715b40be0, fd=14, conn_time=152416) at ../../../../os/connection.c:739
#14 0x000055e7136e46e1 in EstablishNewConnections (clientUnused=0x0, closure=0x4) at ../../../../os/connection.c:815
#15 0x000055e71369e871 in ProcessWorkQueue () at ../../../../dix/dixutils.c:523
#16 0x000055e7136df713 in WaitForSomething (are_ready=0) at ../../../../os/WaitFor.c:210
#17 0x000055e71368e274 in Dispatch () at ../../../../dix/dispatch.c:422
#18 0x000055e71369d649 in dix_main (argc=11, argv=0x7ffd018db218, envp=0x7ffd018db278) at ../../../../dix/main.c:287
#19 0x000055e71367e088 in main (argc=11, argv=0x7ffd018db218, envp=0x7ffd018db278) at ../../../../dix/stubmain.c:34

A look at os/access.c in git master shows it still has the buggy code. It can be tested even without knowing how to build xorg in the equivalent snippet:
#include <inttypes.h>
#include <stdlib.h>
#include <stdio.h>
#include <string.h>

int main() {
  char *cmd = strdup(":a");
  printf("%p\n", cmd);
  cmd = strtok(cmd, ":");
  printf("%p\n", cmd);
  free(cmd);  // Crash. *** Error in `./a.out': free(): invalid pointer: ...
  return 0;

It looks like this problem was introduced in https://lists.freedesktop.org/archives/xorg-devel/2015-December/048259.html aka https://cgit.freedesktop.org/xorg/xserver/commit/?id=e156c0ccb530897d3a428255bd5585f7ea7b9b41. The code should not reuse cmd for the strtok output, so it can free the original strdup'ed string -- see attached patch.
Comment 1 Michel Dänzer 2017-12-05 16:44:31 UTC
Good catch, can you send the patch to the xorg-devel mailing list for review per


Comment 2 kailoran+fd 2017-12-06 11:20:54 UTC
Done, I think (in moderation queue currently).
Comment 3 GitLab Migration User 2018-12-13 22:38:32 UTC
-- GitLab Migration Automatic Message --

This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity.

You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/xorg/xserver/issues/527.

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.