| Summary: | XSetCommand integer overflow | ||
|---|---|---|---|
| Product: | xorg | Reporter: | Konstantin SKliarov <kos.sklyar91> |
| Component: | Lib/Xlib | Assignee: | X.Org Security <xorg_security> |
| Status: | RESOLVED FIXED | QA Contact: | Xorg Project Team <xorg-team> |
| Severity: | critical | ||
| Priority: | high | Keywords: | security |
| Version: | git | ||
| Hardware: | All | ||
| OS: | Linux (All) | ||
| Whiteboard: | |||
| i915 platform: | i915 features: | ||
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. If you can control argv, you've already got quite a lot of control over the host process anyway ... 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. 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. 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-
gram.)
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.
|
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.
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. Regards, Konstantin Skliarov