Bug 94281 - XSetCommand integer overflow
Summary: XSetCommand integer overflow
Alias: None
Product: xorg
Classification: Unclassified
Component: Lib/Xlib (show other bugs)
Version: git
Hardware: All Linux (All)
: high critical
Assignee: X.Org Security
QA Contact: Xorg Project Team
Keywords: security
Depends on:
Reported: 2016-02-24 17:22 UTC by Konstantin SKliarov
Modified: 2018-03-30 23:00 UTC (History)
0 users

See Also:
i915 platform:
i915 features:


Description Konstantin SKliarov 2016-02-24 17:22:50 UTC
Dear Xorg security team

It has been noticed that in src/SetHints.c from git://anongit.freedesktop.org/xorg/lib/libX11 repo

206 int
207 XSetCommand (
208     Display *dpy,
209     Window w,
210     char **argv,
211     int argc)
212 {
213     register int i;
214     register int nbytes;
215     register char *buf, *bp;
216     for (i = 0, nbytes = 0; i < argc; i++) {
217         nbytes += safestrlen(argv[i]) + 1;
218     }
219     if ((bp = buf = Xmalloc(nbytes))) {
220         /* copy arguments into single buffer */
221         for (i = 0; i < argc; i++) {
222         if (argv[i]) {
223             (void) strcpy(bp, argv[i]);
224             bp += strlen(argv[i]) + 1;
225         }
226         else
227             *bp++ = '\0';
228         }
229         XChangeProperty (dpy, w, XA_WM_COMMAND, XA_STRING, 8,
230                  PropModeReplace, (unsigned char *)buf, nbytes);
231         Xfree(buf);
232     }
233     return 1;
234 }

[Line 217]: nbytes Integer overflow -> Heap overrun

[Line 223-224, 227]: bp pointer can be controlled and lead to controlled data writing with additional ability to write zero-byte [*bp++ = '\0'] to controlled address

The call graph is:
  +-< XSetCommand
    +-< Atom
    +-< XSetStandardProperties
    +-< XSetWMProperties
    | +-< XmbSetWMProperties
    | +-< Xutf8SetWMProperties

argv and argc parameters are not sanitized elsewhere on their way to XSetCommand.

Execution can lead to denial of service or potential arbitrary code execution.

Thank you.


Konstantin Skliarov
Comment 1 Alan Coopersmith 2016-02-24 17:56:45 UTC
Will any kernel we support allow enough arguments through argc/argv to hit this
overflow condition?  I don't see any with an ARG_MAX value >= 2 gigabytes on
http://www.in-ulm.de/~mascheck/various/argmax/ though it only has Linux up to
2.6 releases.
Comment 2 Daniel Stone 2016-02-25 08:57:26 UTC
If you can control argv, you've already got quite a lot of control over the host process anyway ...
Comment 3 Alan Coopersmith 2016-02-25 21:32:44 UTC
True, this would really only be a concern for people who still haven't learned
not to make their X11 applications setuid and to instead use setuid-helpers for
the small bits requiring privilege, and even then only on systems with
ARGMAX >= INT_MAX, for which no known instances have been reported yet.

So sure, we could clean up & harden this code, to protect against unknown or
future systems with ARGMAX >= INT_MAX, but I see no way to have any denial of
service or potential arbitrary code execution on any known system today, so
wouldn't call it a security vulnerability or issue a security alert for it.
Comment 4 Konstantin SKliarov 2016-02-26 08:17:29 UTC
Just one thing. You rely on the kernel's exec ARGMAX.
But we are discussing public API of shared library. So, I assume, there can be other unrestricted sources of input. However, I am not familiar with X11.
Comment 5 Alan Coopersmith 2016-02-26 17:57:48 UTC
Oh, so you are arguing that programs may be passing something other than
the argc/argv from main() to XSetCommand()?

I suppose the documentation allows for that, as the man page says:
       The XSetCommand function sets the command and arguments used to invoke
       the application.  (Typically, argv is the argv array of your main pro-
and "typically" allows for other uses in rarer cases.

Still, without any evidence of known uses of this to pass unlimited user
controlled data across a privilege or trust boundary, I'm not ready to
call it a security vulnerability.
Comment 6 Alan Coopersmith 2018-03-25 02:49:50 UTC

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.