LWN.net Logo

Numerous security issues in X Window System clients

From:  Alan Coopersmith <alan.coopersmith-AT-oracle.com>
To:  xorg-announce-AT-lists.x.org
Subject:  [ANNOUNCE] X.Org Security Advisory: Protocol handling issues in X Window System client libraries
Date:  Thu, 23 May 2013 08:05:22 -0700
Message-ID:  <20130523150521.GA26044@also.us.oracle.com>
Cc:  xorg-AT-lists.x.org, xorg-devel-AT-lists.x.org
Archive-link:  Article, Thread

X.Org Security Advisory:  May 23, 2013
Protocol handling issues in X Window System client libraries
============================================================

Description:
============

Ilja van Sprundel, a security researcher with IOActive, has discovered
a large number of issues in the way various X client libraries handle
the responses they receive from servers, and has worked with X.Org's
security team to analyze, confirm, and fix these issues.

Most of these issues stem from the client libraries trusting the server
to send correct protocol data, and not verifying that the values will
not overflow or cause other damage.   Most of the time X clients & servers
are run by the same user, with the server more privileged from the clients,
so this is not a problem, but there are scenarios in which a privileged
client can be connected to an unprivileged server, for instance, connecting
a setuid X client (such as a screen lock program) to a virtual X server
(such as Xvfb or Xephyr) which the user has modified to return invalid
data, potentially allowing the user to escalate their privileges.

The X.Org security team would like to take this opportunity to remind
X client authors that current best practices suggest separating code
that requires privileges from the GUI, to reduce the attack surface of
issues like this.

The vulnerabilities include:

- integer overflows calculating memory needs for replies

    These calls do not check that their calculations for how much memory
    is needed to handle the returned data have not overflowed, so can
    result in allocating too little memory and then writing the returned
    data past the end of the allocated buffer.

    * CVE-2013-1981: libX11 1.5.99.901 (1.6 RC1) and earlier
      Affected functions:  XQueryFont(), _XF86BigfontQueryFont(),
          XListFontsWithInfo(), XGetMotionEvents(), XListHosts(),
          XGetModifierMapping(), XGetPointerMapping(), XGetKeyboardMapping(),
          XGetWindowProperty(), XGetImage()

    * CVE-2013-1982: libXext 1.3.1 and earlier
      Affected functions:  XcupGetReservedColormapEntries(),
          XcupStoreColors(), XdbeGetVisualInfo(), XeviGetVisualInfo(),
          XShapeGetRectangles(), XSyncListSystemCounters()

    * CVE-2013-1983: libXfixes 5.0 and earlier
      Affected functions:  XFixesGetCursorImage()

    * CVE-2013-1984: libXi 1.7.1 and earlier
      Affected functions:  XGetDeviceControl(), XGetFeedbackControl(),
          XGetDeviceDontPropagateList(), XGetDeviceMotionEvents(),
          XIGetProperty(), XIGetSelectedEvents(), XGetDeviceProperties(),
          XListInputDevices()

    * CVE-2013-1985: libXinerama 1.1.2 and earlier
      Affected functions:  XineramaQueryScreens()

    * CVE-2013-2062: libXp 1.0.1 and earlier
      Affected functions:  XpGetAttributes(), XpGetOneAttribute(),
          XpGetPrinterList(), XpQueryScreens()

    * CVE-2013-1986: libXrandr 1.4.0 and earlier
      Affected functions:  XRRQueryOutputProperty(), XRRQueryProviderProperty()
         [XRRQueryProviderProperty() was introduced in libXrandr 1.4.0 and is
          not found in 1.3.2 and older releases.]

    * CVE-2013-1987: libXrender 0.9.7 and earlier
      Affected functions:  XRenderQueryFilters(), XRenderQueryFormats(),
          XRenderQueryPictIndexValues()

    * CVE-2013-1988: libXRes 1.0.6 and earlier
      Affected functions:  XResQueryClients(), XResQueryClientResources()

    * CVE-2013-2063: libXtst 1.2.1 and earlier
      Affected functions:  XRecordGetContext()

    * CVE-2013-1989: libXv 1.0.7 and earlier
      Affected functions:  XvQueryPortAttributes(), XvListImageFormats(),
          XvCreateImage()

    * CVE-2013-1990: libXvMC 1.0.7 and earlier
      Affected functions:  XvMCListSurfaceTypes(), XvMCListSubpictureTypes()

    * CVE-2013-1991: libXxf86dga 1.1.3 and earlier
      Affected functions:  XDGAQueryModes(), XDGASetMode()

    * CVE-2013-1992: libdmx 1.1.2 and earlier
      Affected functions:  DMXGetScreenAttributes(), DMXGetWindowAttributes(),
          DMXGetInputAttributes()

    * CVE-2013-2064: libxcb 1.9 and earlier
      Affected functions:  read_packet()

    * CVE-2013-1993: libGLX in Mesa 9.1.1 and earlier
      Affected functions:  XF86DRIOpenConnection(), XF86DRIGetClientDriverName()

    * CVE-2013-1994: libchromeXvMC & libchromeXvMCPro in openChrome 0.3.2
      and earlier
      Affected functions:  uniDRIOpenConnection(), uniDRIGetClientDriverName()

- sign extension issues calculating memory needs for replies

    These calls do not check that their calculations for how much memory
    is needed to handle the returned data have not had sign extension
    issues when converting smaller integer types to larger ones, leading
    to negative numbers being used in memory size calculations that can
    result in allocating too little memory and then writing the returned
    data past the end of the allocated buffer.

    * CVE-2013-1995: libXi 1.7.1 and earlier
      Affected functions:  XListInputDevices()

    * CVE-2013-1996: libFS 1.0.4 and earlier
      Affected functions:  FSOpenServer()

- buffer overflows due to not validating length or offset values in replies

    These calls do not check that the lengths and/or indexes returned by the
    server are within the bounds specified by the caller or the bounds of the
    memory allocated by the function, so could write past the bounds of
    allocated memory when storing the returned data.

    * CVE-2013-1997: libX11 1.5.99.901 (1.6 RC1) and earlier
      Affected functions:  XAllocColorCells(), _XkbReadGetDeviceInfoReply(),
          _XkbReadGeomShapes(), _XkbReadGetGeometryReply(), _XkbReadKeySyms(),
          _XkbReadKeyActions(), _XkbReadKeyBehaviors(), _XkbReadModifierMap(),
          _XkbReadExplicitComponents(), _XkbReadVirtualModMap(),
          _XkbReadGetNamesReply(), _XkbReadGetMapReply(), _XimXGetReadData(), 
          XListFonts(), XListExtensions(), XGetFontPath()

    * CVE-2013-1998: libXi 1.7.1 and earlier
      Affected functions:  XGetDeviceButtonMapping(), _XIPassiveGrabDevice(),
          XQueryDeviceState()

    * CVE-2013-2066: libXv 1.0.7 and earlier
      Affected functions:  XvQueryPortAttributes()

    * CVE-2013-1999: libXvMC 1.0.7 and earlier
      Affected functions:  XvMCGetDRInfo()

    * CVE-2013-2000: libXxf86dga 1.1.3 and earlier
      Affected functions:  XDGAQueryModes(), XDGASetMode()

    * CVE-2013-2001: libXxf86vm 1.1.2 and earlier
      Affected functions:  XF86VidModeGetGammaRamp()

    * CVE-2013-2002: libXt 1.1.3 and earlier
      Affected functions:  _XtResourceConfigurationEH()

- integer overflows parsing user-specified files

    These calls do not check that their calculations for how much memory
    is needed to handle the data being read have not overflowed, so can
    result in allocating too little memory and then writing the returned
    data past the end of the allocated buffer.

    * CVE-2013-1981: libX11 1.5.99.901 (1.6 RC1) and earlier
      Affected functions:  LoadColornameDB(), XrmGetFileDatabase(),
          _XimParseStringFile(), TransFileName()

    * CVE-2013-2003: libXcursor 1.1.13 and earlier
      Affected functions:  _XcursorFileHeaderCreate()

- unbounded recursion parsing user-specified files

    These calls read in files and handle C-style '#include' directives
    to include other files, and have no limit for how many levels deep
    they will go, including allowing files to #include themselves, until
    the stack overflows from the recursive function calling patterns.

    * CVE-2013-2004: libX11 1.5.99.901 (1.6 RC1) and earlier
      Affected functions:  GetDatabase(), _XimParseStringFile()

- memory corruption due to unchecked return values

    These calls assume that pointers are properly initialized by the
    XGetWindowProperty() function and don't check for failure of the
    function to return a valid window property, which can lead to
    use of uninitialized pointers for reading, writing, or passing to
    functions such as free().   XGetWindowProperty() in libX11 1.5.99.901
    (1.6RC1) and earlier did not ensure returned pointers were initialized
    to NULL when returning a failure (this is fixed in libX11 1.5.99.902
    and later).

    * CVE-2013-2005: libXt 1.1.3 and earlier
      Affected functions:  ReqCleanup(), HandleSelectionEvents(),
          ReqTimedOut(), HandleNormal(), HandleSelectionReplies()

Affected Versions
=================

X.Org believes all prior versions of these libraries contain these
flaws, dating back to their introduction.

Versions of the X libraries built on top of the Xlib bridge to the XCB 
framework are vulnerable to fewer issues than those without, due to the
added safety and consistency assertions in the XCB calls to read data
from the network, but most of these vulnerabilities are not caught by
those checks.

Fixes
=====

Fixes are available in git commits and patches which will be listed
on http://www.x.org/wiki/Development/Security/Advisory-2013-...
when this advisory is released.

Fixes will also be included in these module releases from X.Org:

    libX11 1.5.99.902 (1.6 RC2)
    libXcursor 1.1.14
    libXext 1.3.2
    libXfixes 5.0.1
    libXi 1.7.2
    libXinerama 1.1.3
    libXp 1.0.2
    libXrandr 1.4.1
    libXrender 0.9.8
    libXRes 1.0.7
    libXv 1.0.8
    libXvMC 1.0.8
    libXxf86dga 1.1.4
    libXxf86vm 1.1.3
    libdmx 1.1.3
    libxcb 1.9.1
    libFS 1.0.5
    libXt 1.1.4

or releases to be determined from our sister projects:
    xf86-video-openchrome    OpenChrome project - http://www.openchrome.org/
    Mesa                     Mesa3D project - http://www.mesa3d.org/

Thanks
======

X.Org thanks Ilja van Sprundel of IOActive for reporting these issues to our
security team and assisting them in understanding them and evaluating our
fixes, and Alan Coopersmith of Oracle for coordinating the X.Org response and
developing the fixes for these issues.

-- 
	-Alan Coopersmith-              alan.coopersmith@oracle.com
	  X.Org Security Response Team - xorg-security@lists.x.org
_______________________________________________
xorg-announce mailing list
xorg-announce@lists.x.org
http://lists.x.org/mailman/listinfo/xorg-announce


(Log in to post comments)

Numerous security issues in X Window System clients

Posted May 23, 2013 23:04 UTC (Thu) by FranTaylor (guest, #80190) [Link]

Surprise surprise, using network connections for no valid reason is not exactly a winning strategy.

Remote X has never worked right and yet we all must endure security vulnerabilities because of irrational desires for stuff that doesn't provide any benefit.

Numerous security issues in X Window System clients

Posted May 23, 2013 23:32 UTC (Thu) by drag (subscriber, #31333) [Link]

X Windows doesn't use network connections for no valid reason. It uses local unix sockets and only network connections when explicitly configured to do so.

The problems stem from bugs in the client libraries used to manage the complex protocol.

Numerous security issues in X Window System clients

Posted May 23, 2013 23:52 UTC (Thu) by rqosa (subscriber, #24136) [Link]

But for many years now, the default X server configuration does not allow network connections, but instead uses AF_LOCAL only — and even Wayland is using AF_LOCAL sockets as its IPC mechanism (according to the documentation). So you can't blame this on the use of network connections.

Really, any code that does IPC (regardless of what IPC mechanism is used) with an untrusted process must be carefully written to make no false assumptions about what the other process sends; there's no way around it.

Numerous security issues in X Window System clients

Posted May 24, 2013 0:04 UTC (Fri) by zlynx (subscriber, #2285) [Link]

Where does this myth that X is somehow different and special come from?

EVERY major operating system now uses a display server and you may as well complain about DWM or Quartz.

Numerous security issues in X Window System clients

Posted May 24, 2013 0:23 UTC (Fri) by Cyberax (✭ supporter ✭, #52523) [Link]

Quartz and Android's SurfaceFlinger do NOT use network for drawing, only for event dispatching and synchronization. That makes them radically simpler and more secure.

The same goes for Wayland and Weston.

Numerous security issues in X Window System clients

Posted May 24, 2013 0:35 UTC (Fri) by dig (guest, #91108) [Link]

X Windows does _not_ use network for drawing, but only to exchange window details like position, size and atoms.

X protocol isn't complicated, but only verbose after all these years. I'm waiting to see how Wayland protocol will look like and behave after 20 years...

Numerous security issues in X Window System clients

Posted May 24, 2013 0:48 UTC (Fri) by Cyberax (✭ supporter ✭, #52523) [Link]

> X Windows does _not_ use network for drawing, but only to exchange window details like position, size and atoms.
Actually, it does.

With lots of ways to do it, from simple zero-width lines to antialiased trapezeoids in X-render. With lots of complicated code which runs in the X-server, with the root privileges. Here's documentation for some of it: http://tronche.com/gui/x/xlib/graphics/

In short, it's been known for ages that the X server is about as holey as Swiss cheese.

Numerous security issues in X Window System clients

Posted May 24, 2013 1:44 UTC (Fri) by dig (guest, #91108) [Link]

Ok, let me paraphrase: modern X windows applications mostly does not use network for drawing ;)

> In short, it's been known for ages that the X server is about as holey as Swiss cheese.

And what you suggest as alternative? Wayland's security model?

Numerous security issues in X Window System clients

Posted May 24, 2013 1:47 UTC (Fri) by Cyberax (✭ supporter ✭, #52523) [Link]

> Ok, let me paraphrase: modern X windows applications mostly does not use network for drawing ;)
Yet they are still in the protocol and they still have to be supported by the X server. And X-Render stuff is actually still used by tons of applications.

> And what you suggest as alternative? Wayland's security model?
Yep. Having a private X server for each application is also an interesting idea.

Numerous security issues in X Window System clients

Posted May 24, 2013 6:58 UTC (Fri) by butlerm (subscriber, #13312) [Link]

> Yep. Having a private X server for each application is also an interesting idea.

Why wouldn't we just have Xlib or XCBlib or whatever dynamically load a library that implements X in process then, instead of engaging in relatively pointless IPC? The lower level server would be handling all the compositing and arbitration.

Numerous security issues in X Window System clients

Posted May 24, 2013 22:59 UTC (Fri) by rqosa (subscriber, #24136) [Link]

> Why wouldn't we just have Xlib or XCBlib or whatever dynamically load a library that implements X in process then, instead of engaging in relatively pointless IPC?

Because it's still useful in many cases (e.g. between machines on the same LAN) to run remote X clients over an SSH tunnel.

Numerous security issues in X Window System clients

Posted May 25, 2013 0:10 UTC (Sat) by butlerm (subscriber, #13312) [Link]

> Because it's still useful in many cases (e.g. between machines on the same LAN) to run remote X clients over an SSH tunnel.

That is why you would dynamically load it - use a library derived from the X server code to draw into something like a Wayland frame buffer when local, and use the X wire protocol when using an X server over a network connection. As long as you are on the same machine, a large class of X applications should run much faster, with a higher degree of security, and so on.

Even without a compositor, this is arguably the way it should have been done from the beginning. What is the point of protecting access to the frame buffer on a typical embedded system? It just slows things down.

Numerous security issues in X Window System clients

Posted May 25, 2013 0:49 UTC (Sat) by dlang (✭ supporter ✭, #313) [Link]

Every replacement system performs better when it's first introduced. It's simple and doesn't yet take into account all the real-world corner cases that users run into.

the question is how much do things slow down by the time they are really ready for users. Is the result really still a lot faster than the old system (which may have picked up some optimizations along the way)

There's also the question of if the difference in speed matters.

shaving 0.1 sec off of a one hour job makes no difference, even if it saves 99% of the time that one particular function takes.

in some cases, it's comparing apples to oranges, for example, see the raspberry pi post where they introduce wayland for the pi and crow about how much faster it is. It's faster because the wayland implementation does the acceleration in the GPU while the X implementation does it in the ARM core. If you were to update the X implementation the same way, the difference would shrink drastically.

Numerous security issues in X Window System clients

Posted May 28, 2013 11:59 UTC (Tue) by daniels (subscriber, #16193) [Link]

in some cases, it's comparing apples to oranges, for example, see the raspberry pi post where they introduce wayland for the pi and crow about how much faster it is. It's faster because the wayland implementation does the acceleration in the GPU while the X implementation does it in the ARM core. If you were to update the X implementation the same way, the difference would shrink drastically.

Yes, but a large part of the actual point was that it's not actually possible to do this with X. So it's more like comparing a full-blown 2D compositor to an orange. If you want to use 2D compositing to represent your scene graph, your only choice with X today is to not use compositing. So you don't get any effects, you get loads of flashing and tearing and incomplete results, etc. It would also be an enormous - enormous - amount of code relative to the Weston backend.

I'd like someone to do it, because it would still make for a really compelling comparison video.

Numerous security issues in X Window System clients

Posted May 28, 2013 20:35 UTC (Tue) by ndye (subscriber, #9947) [Link]

So [the net result is no] effects, [just] loads of flashing and tearing and incomplete results, [at the cost of] enormous code relative to the Weston backend.

I'd [still] like someone to do it [for the] compelling comparison video.

And with such motivators, somebody's sure to jump right in. (Heh!)

Numerous security issues in X Window System clients

Posted May 25, 2013 5:27 UTC (Sat) by rqosa (subscriber, #24136) [Link]

> use a library derived from the X server code to draw into something like a Wayland frame buffer when local, and use the X wire protocol when using an X server over a network connection.

Well, that could work, but…

> a large class of X applications should run much faster, with a higher degree of security

…I don't see how that improves the security any, since an X server on Wayland shouldn't need to run with elevated privileges. So you can prevent the kind of privilege escalation scenario described in the article by, for example, not putting the setuid flag on the client program itself but instead using a setuid wrapper that spawns both the client and a server instance for the client to connect to (thus preventing the client from connecting to an arbitrary server unless the client is running with regular privileges).

And as for running faster, the programs where that's important probably will be using direct rendering anyway.

Numerous security issues in X Window System clients

Posted May 25, 2013 8:35 UTC (Sat) by Cyberax (✭ supporter ✭, #52523) [Link]

1) Less code - less potential for bugs.
2) Private X server would be working within the context of a single application, so bugs and crashes in it won't affect other applications.
3) Each application can be further restricted by various sandboxing technologies.

Numerous security issues in X Window System clients

Posted May 25, 2013 9:11 UTC (Sat) by dlang (✭ supporter ✭, #313) [Link]

with dozens of X servers running, what actually controls access to the hardware?

Numerous security issues in X Window System clients

Posted May 25, 2013 9:12 UTC (Sat) by Cyberax (✭ supporter ✭, #52523) [Link]

The entity responsible for hardware control - the kernel.

Numerous security issues in X Window System clients

Posted May 25, 2013 19:13 UTC (Sat) by rqosa (subscriber, #24136) [Link]

> Private X server would be working within the context of a single application, so bugs and crashes in it won't affect other applications.

Actually, private X servers are what I was referring to here:

> > a setuid wrapper that spawns both the client and a server instance for the client to connect to

(Basically, my point was that if it's possible to run an arbitrary amount of X-on-Wayland servers with arbitrary privileges on each server instance, then it's possible to prevent all X client-to-server and server-to-client privilege escalation scenarios.)

Numerous security issues in X Window System clients

Posted May 24, 2013 7:27 UTC (Fri) by renox (subscriber, #23785) [Link]

> Ok, let me paraphrase: modern X windows applications mostly does not use network for drawing ;)

You're doubly wrong:
1) you can configure KDE|Qt to use XRender.
2) as Cyberax said, even the oldest "draw stipple line" that noone use are still part of the protocol, of course this matter more for the server than for the clients.

Numerous security issues in X Window System clients

Posted May 24, 2013 3:50 UTC (Fri) by kevinm (guest, #69913) [Link]

> In short, it's been known for ages that the X server is about as holey as Swiss cheese.

That may be true, but this particular set of flaws is not about the X server - its about the X client libraries.

Numerous security issues in X Window System clients

Posted May 25, 2013 18:11 UTC (Sat) by viro (subscriber, #7872) [Link]

... and there you go, injecting reality into a perfectly good outraged handwaving.

In the meanwhile, the "fixes" in question are nearly pointless and seriously broken. Nearly pointless because compromised _server_ doesn't need to bugger the memory of client with overflows; usually it can do it either with ptrace (when on the same box) or via ssh + ptrace (when the client runs on remote box). Seriously broken - hell knows why, presumably it didn't get reviewed (embargo?).

Thursday fun: after grabbing these packages on a debian box, xine started to coredump on startup. After digging through the stack trace, it turned out that something had been calling XextAddDisplay with NULL dpy, with obvious fun results. Trace went through libxext and libxvmc, both included into updated, so the next step had been to compare source before and after. Diff looked like a pile of moderately pointless hardening, which might've caused that (if some test had been too eager and recovery from the misdetected error sufficiently bogus). However, it contained some bits outside of error paths, and those were worth looking at first. Like that one:

strncpy(*name,tmpBuf,rep.nameLen);
+ name[rep.nameLen - 1] = '\0';

Cargo-cult programming? Innocent braino that simply hadn't been caught before it hit the tree? Hell knows, in either case it's obviously bogus (name is char ** according to the older line, which makes the '\0' in the added one an obfuscated NULL) and very likely to cause the symptoms observed - by the look of that code, name is going to be an address of local variable in caller's stack frame, so it's quite plausible that dpy sat in a stack word nearby and got zeroed out by that assignment. Or the one added next to it (s/name/busID/). While we are at it, error cleanup changes right next to that place in diff were obviously not reviewed either -

XFree(*name);
*name = NULL;
XFree(*busID);
*name = NULL;

spells "cut'n'paste lossage". OK, fixed both, rebuilt libxvmc, xine's working again. Next question - was that Debian-specific fuckup or was it in mainline? git clone, git log -p and it turns out to be the latter... Next step was to catch somebody connected to upstream (airlied hadn't ducked in time) and point to the idiocy in question. Total time spent: two hours...

For what it's worth, the interesting (and potentially very nasty) story here is that this demonstrates a damn good way to insert a hole into some tree - pile up a lot of obvious stuff ("hardening" of that sort, etc.) and bury the backdoor in pages upon pages of that. Then claim a bunch of CVEs and feed the fixes to projects in question, demanding an embargo to reduce the amount of people who'll be reviewing that pile.

I do not believe that this is what happened here (IOW, that this fuckup had been malicious), but it's only a matter of time before somebody pulls that off for real; it's a very tempting attack vector. Review is blunted by combination of sheer boredom (lot of trivial stuff to look through), presumed good intent of the patchset (security improvements, no less), possibility to distribute the hole between several libraries and reduction of the available reviewers by embargo. Deniability is also pretty high, even if attacker gets caught afterwards...

Numerous security issues in X Window System clients

Posted May 26, 2013 2:41 UTC (Sun) by drag (subscriber, #31333) [Link]

> ... and there you go, injecting reality into a perfectly good outraged handwaving.

> In the meanwhile, the "fixes" in question are nearly pointless and seriously broken. Nearly pointless because compromised _server_ doesn't need to bugger the memory of client with overflows; usually it can do it either with ptrace (when on the same box) or via ssh + ptrace (when the client runs on remote box). Seriously broken - hell knows why, presumably it didn't get reviewed (embargo?).

What I think you are missing is that it's common to run certain setuid root binaries or run programs via sudo.

I think a large part of that has been mitigated on full-flavored desktops like Gnome, but with the popularity of 'lite'er desktops then a lot of administrative duties is done via sudo and whatnot.

So any sort of flaw in x libraries means local and even remote root exploit.

Numerous security issues in X Window System clients

Posted May 26, 2013 3:36 UTC (Sun) by viro (subscriber, #7872) [Link]

It presupposes a compromised X server on the box you are sitting at. Which, at the absolute minimum, is as bad as attacker physically stumbling across your unattended terminal.

Sure, it might be yourself trying to escalate from limited sudo access to e.g. unrestricted root shell, so it's not absolutely pointless, but it's seriously oversold in the announcement. Note that the breakage introduced (and not caught on review) is 100% real and its impact is not a lot safer than "what if there's no NUL in that string?" crap it fixes.

The real problem here is abuse of embargo for PR purposes. The brainos in question are trivial - everyone had done something like that by accident. Many times. That's what code review is for. And in this case it was prevented by embargo ;-/

Numerous security issues in X Window System clients

Posted May 26, 2013 3:56 UTC (Sun) by viro (subscriber, #7872) [Link]

BTW, now that I've looked at that thing in more details... It's not foolproof - rep.nameLen and rep.busIDLen are u32, and on a 64bit box it's quite possible to have Xmalloc on both succeeding, with sum wrapping around.
At which point
strncpy(*name,tmpBuf,rep.nameLen);
(*name)[rep.nameLen - 1] = '\0';
strncpy(*busID,tmpBuf+rep.nameLen,rep.busIDLen);
(*busID)[rep.busIDLen - 1] = '\0';
will do wonders if rep.nameLen is large (exceeds the size of tmpBuf) *and* reply has NUL within the first few bytes. The first strncpy() succeeds nicely, the second one uses a pointer well beyond tmpBuf end as the source...

The damn thing really looks like it hadn't been reviewed at all.

Numerous security issues in X Window System clients

Posted May 26, 2013 18:41 UTC (Sun) by drag (subscriber, #31333) [Link]

> It presupposes a compromised X server on the box you are sitting at. Which, at the absolute minimum, is as bad as attacker physically stumbling across your unattended terminal.

There are X servers that do not require setuid root permissions to operate.

% which Xephyr
/usr/bin/Xephyr
% ls -altr `which Xephyr`
-rwxr-xr-x 1 root root 2101072 Apr 17 00:53 /usr/bin/Xephyr
% cp `which Xephyr` ~/
% ~/Xephyr

This works just fine. Besides Xephyr there is a wide variety of other X11 servers that can be copied around easily. One thing that I use quite often is TigerVNC's X11 server for doing remote desktop since for my purposes the VNC protocol is much more responsive and useful then X11 protocol. This sort of thing is quite common to use.

> Sure, it might be yourself trying to escalate from limited sudo access to e.g. unrestricted root shell, so it's not absolutely pointless, but it's seriously oversold in the announcement. Note that the breakage introduced (and not caught on review) is 100% real and its impact is not a lot safer than "what if there's no NUL in that string?" crap it fixes.

It goes to show how woefully people over estimate the security of the Linux desktop if somebody who programs like that is able to trivially find dozens of problems just by being the first one to actually read the code.

Numerous security issues in X Window System clients

Posted May 26, 2013 20:38 UTC (Sun) by viro (subscriber, #7872) [Link]

WTF does that have to do with suid-root? Compromised X server in front of you can bloody well feed arbitrary keystrokes to your shell/browsers/etc. and hide the resulting output from you. Or it can pretend that you've got screensaver kick in and wait for your password, etc. All keyboard/mouse/screen IO goes through it; it can do everything you can do.

Escalation from limited access via /etc/sudoers to full root shell is a different story, but it requires a setup that is probably less common than you think. Again, I certainly agree that one needs to validate the input via any kind of IPC. Something similar to these patches is needed; no arguments here. Severity of impact, OTOH, is quite overblown.

As for reading the source... Judging by the quality of patch, I strongly suspect that this guy hadn't actually done that - grepped for strncpy and friends and did cargo-cult kind of "fixing". Sure, that crap needed fixing, even though the impact is nowhere near as dramatic as you seem to imply (and fixes are obviously not good enough). No problem with that; what I *do* have a problem with is the amount of PR circus in the whole story. Shit happens; let the one who had never fucked up in a patch throw the first stone. Hyping the patch from "here's a bunch of proposed low-hanging fixes for unsafe strncpy() uses, please review and apply" to "28 CVEs!!!1!!! Security holes!!1!!!1!!!!! Embargo!!! (so that it would hit the websites all at once, promoting the hell out of the patch author)", OTOH, is a different story, especially since the embargo has prevented spotting the holes in the patch in question.

Numerous security issues in X Window System clients

Posted May 27, 2013 1:29 UTC (Mon) by drag (subscriber, #31333) [Link]

> WTF does that have to do with suid-root?

Because you seemed to think that a providing a compromised X Server would be some sort of barrier to the exploit unless the attacker already had root access. Xfree X server requires suid-root to run and I was guessing that is what you were thinking about. I was just pointing out that you don't need those types of permissions to run most X servers.

> As for reading the source... Judging by the quality of patch, I strongly suspect that this guy hadn't actually done that - grepped for strncpy and friends and did cargo-cult kind of "fixing".

Oh. So these vulnerabilities are even more trivial to find then I originally thought. It's all a bit depressing altogether. Another bit of evidence that can be used to show that he Android guys were right to drop all this X/traditional *nix nonsense right from the get-go.

Numerous security issues in X Window System clients

Posted May 27, 2013 2:04 UTC (Mon) by viro (subscriber, #7872) [Link]

> Because you seemed to think that a providing a compromised X Server would
> be some sort of barrier to the exploit unless the attacker already had
> root access. Xfree X server requires suid-root to run and I was guessing
> that is what you were thinking about. I was just pointing out that you
> don't need those types of permissions to run most X servers.

Huh? For one thing, the text above makes no sense whatsoever. For another, just what in the postings upthread might have possibly been interpreted in such a way? Granted, I'm not a native speaker, but...

Numerous security issues in X Window System clients

Posted May 28, 2013 18:17 UTC (Tue) by acoopersmith (subscriber, #72107) [Link]

> Nearly pointless because compromised _server_ doesn't need to bugger the memory of client with overflows;

The point of these is not to protect a user from a compromised server - it's to protect a system from a malicious local unprivileged user compromising a privileged client using their own customized server in order to gain the privileges of that client. For people running Linux on a box in which they're the only user, this is pretty much a non-problem (as long as you're sure no remote attacker could ever break in and then use a bug such as this to break in further), but for sites like schools & businesses using servers with many users, it could be worse.

While the common case of an X server is a privileged process talking directly to the hardware, it's simply a binary protocol - any program that can open a socket and respond in the fashion documented in the X protocol spec can act as an X server if you set your $DISPLAY to point to it. In the process of testing these I was able to corrupt memory in the clients with an “X server” consisting of less than 1000 lines of C code.

> presumably it didn't get reviewed (embargo?).

Yes, the embargo made it difficult to get review for the sheer number of patches involved and you'll note many of the commits missing a “Reviewed-by” tag. We got good coverage of the libX11 & libXi changes, not so much of the rest of the libraries.

Several of us also tested with the libraries on our desktops for a few weeks before release, but I don't think any of us use video players with libXvmc support. (I also missed an XKB bug because I use the default US/English keyboard layout and don't change layouts at runtime - our QA team in Oracle's Prague office found that quite quickly since failing to load a cz layout is much more noticeable to them, and Julien Cristau had developed a fix for Debian at the same time.) We also ran the automated test suites we have, but those mostly cover the libraries from the X11R5/6 timeframe in the mid-90's, not the later extensions, since no one has funded the tedious work of test case writing since then, and few have volunteered much time to work on that.

You can see from the git commit dates we'd been working on these for about 3 months, and finally figured we'd done as much as we could do on the private/embargoed list, so put them out to the world for wider review & testing. I specifically chose not to cut new stable releases of the libraries until this happened, and am glad to see we've gotten better testing and bugs reported via distros such as Debian who put them out to their users quickly.

> this demonstrates a damn good way to insert a hole into some tree - pile up a lot of obvious stuff ("hardening" of that sort, etc.) and bury the backdoor in pages upon pages of that

Yes, code review will never be perfect, and even small changes can introduce security holes - we've found that in X before when a bunch of code reviewers failed to notice a missing pair of parentheses that led to CVE-2006-0745, and only when Coverity pointed them out did we spot the bug and the implications.

Numerous security issues in X Window System clients

Posted May 28, 2013 21:03 UTC (Tue) by viro (subscriber, #7872) [Link]

Well, yes - that's why I said "nearly pointless" and not "entirely pointless". That's the escalation from suid/sudoers-mediated to root shell scenario, but to make it work you need an elevated-priv application attacker can run that would talk to X server. What would that be on common multi-user setups? IOW, the impact is rather limited. These bugs obviously needed fixing (still do, most likely - arithmetical overflow analysis was obviously incomplete), but IMO an embargo had been a mistake in the best case and PR games in the worst (realistic) one.

FWIW, any application that runs with elevated privs would be better served by isolating the part that needs those and trimming it down as far as possible. With protocol for interaction between the priveleged part and the rest being as simple as possible, precisely to make validation more robust... I'm obviously not saying that there's no userland programs that would violate that principle, but any such program is seriously asking for review and rewrite - it's very likely to contain other holes.

PS: "specifically chose not to cut new stable releases of the libraries until this happened" sounds like we are in agreement re the worth of embargo in this case - my impression from the whole thing is that embargo had nothing to do with protecting vulnerable systems and everything with PR theater wanted by the submitter of those patches...

Numerous security issues in X Window System clients

Posted May 24, 2013 6:51 UTC (Fri) by micka (subscriber, #38720) [Link]

> I'm waiting to see how Wayland protocol will look like and behave after 20 years...

You'll just have to wait 20 years.

Numerous security issues in X Window System clients

Posted May 24, 2013 7:02 UTC (Fri) by geofft (subscriber, #59789) [Link]

What does this have to do with the network, at all? It's about a binary protocol between two components of different privilege. The same class of vulnerabilities could as well exist in D-Bus, in the Windows syscall API, in packet filtering, in communication between Chrome and its tabs, in SATA or SCSI or Android intents or systemd talking to services -- nothing here is specific to X being on the network.

And fundamentally, this entire class of vulnerabilities could equally well exist in every single other display architecture, even though they don't use "network" connections.

Numerous security issues in X Window System clients

Posted May 24, 2013 7:28 UTC (Fri) by ortalo (subscriber, #4654) [Link]

Entirely agreed. I'd go even further: these classes of vulnerability probably still exists in other big pieces of software not as well scrutinized as the major ones. Such examples should make us worry much more about our database clients, our spreadsheets, our mail clients, etc.
Fortunately for open source, these examples give us even more inspiring ideas about the situation with proprietary or industrial software...

Numerous security issues in X Window System clients

Posted May 24, 2013 11:21 UTC (Fri) by ajmacleod (guest, #1729) [Link]

Re: Fran Taylor -- What complete nonsense. Remote X has worked remarkably well for decades and is incredibly useful and flexible. No doubt it harks back to an era where security was less of an issue but still today in the real world it just gets the job done, limitations accepted and understood.

I really wish the clueless idiots who don't understand or use this flexibility would stop shouting loudly about X being a worthless disaster with remote display abilities that nobody uses when these same abilities have been making life simple for so many people for such a long time; we don't usually shout about that because we've taken it for granted for so long.

Numerous security issues in X Window System clients

Posted May 24, 2013 16:12 UTC (Fri) by Wol (guest, #4433) [Link]

And remote X is still used today - I use it!

It's a great way for repurposing old hardware, I use it as an X-terminal while my wife is on the main machine.

The big pain is the Cygwin X server (on Win 7) does not seem to work very well at all.

Cheers,
Wol

Numerous security issues in X Window System clients

Posted May 27, 2013 16:59 UTC (Mon) by FranTaylor (guest, #80190) [Link]

"I still drive my Ford Pinto, my personal experience is proof that there is no danger"

Numerous security issues in X Window System clients

Posted May 27, 2013 15:38 UTC (Mon) by FranTaylor (guest, #80190) [Link]

no desktop integration

no sound

no 3-D

any sophisticated X client is not going to be properly functional without these things

sure you can start an X term but there are many other ways to accomplish this functionality

Numerous security issues in X Window System clients

Posted May 27, 2013 22:31 UTC (Mon) by nix (subscriber, #2304) [Link]

That's an argument made entirely of straw.

Desktop integration works fine if it relies on things like X properties which propagate across remote clients. If it doesn't, I don't care. If sound and 3D don't work, I similarly don't care: what I care about is my huge Emacs / simulation / VM VNC sessions[1] running on the headless machine (with gobs of RAM, a fast set of RAID disks and an SSD cache) sitting in the closet and not on the desktop box with much less RAM and a far slower disk subsystem sitting next to me.

Remote X makes that work. I don't care if I can't run Oolite remotely. That's not the sort of thing remote X is for.

[1] yes, I know these days one can use SPICE, but not on all VM hosts and this wasn't by any means always true

Numerous security issues in X Window System clients

Posted May 27, 2013 23:54 UTC (Mon) by Cyberax (✭ supporter ✭, #52523) [Link]

We're using VNC for this very successfully. We're running large computational loads on Amazon EC2 nodes and often use MATLAB and other applications locally on them.

X is very painful across the continent, but VNC is just fine. Also, VNC allows us to disconnect and reconnect.

Numerous security issues in X Window System clients

Posted May 28, 2013 20:18 UTC (Tue) by nix (subscriber, #2304) [Link]

Where did you get 'across the continent' from? I'm interested in remoting across the *room*: I know X's latency constraints unfortunately prevent WANnish remoting. I'm also interested in scrolling fast, which is utterly impossible with VNC because of its shovel-bitmaps-about approach, as I've mentioned before here at frankly tedious length. VNC probably works for things with big mostly-static canvasses, but not for anything that involves scrolling, particularly not scrolling things with a lot of detail in their windows.

Numerous security issues in X Window System clients

Posted Jun 17, 2013 9:08 UTC (Mon) by robbe (subscriber, #16131) [Link]

The VNC Protocol has the CopyRect encoding, which can handle scrolling very efficiently. Of course the server has to actually use it ... which may be hard, unless for example it is Xvnc getting an XCopyArea request.

Numerous security issues in X Window System clients

Posted Jun 18, 2013 12:42 UTC (Tue) by nix (subscriber, #2304) [Link]

Yeah, the server has to use it -- which means the server can't just throw raw bitmaps around, but has to figure out that this bitmap was just scrolled. Now I suppose it could do this via some sort of bitmap analysis, but that'd be cache-ruinous and a total waste of CPU time compared to just noticing that, say, the toolkit just did a scroll. But... Wayland, AIUI, can't do that, by design.

Numerous security issues in X Window System clients

Posted Jun 18, 2013 21:47 UTC (Tue) by foom (subscriber, #14868) [Link]

I know nothing about Wayland's actual code, but there should be no reason why Wayland could not have a protocol for a client to be able to send two hints to the rendering layer:
1) "I'll explicitly tell you about all scrolls in $this bounding-box in my window."
2) "Hi, I just moved $this rectangle to $there".

Purely as an optimization, for clients which decide to use it. Whether or not it's actually worth implementing is another matter (I'd suspect it is probably not worth it), but I really doubt the design precludes it.

Numerous security issues in X Window System clients

Posted May 28, 2013 21:00 UTC (Tue) by ballombe (subscriber, #9523) [Link]

> sure you can start an X term but there are many other ways to accomplish this functionality

You do not understand. The magic of X remote is that it is always available.
It does not depend on the whim of the sysadmin, of the program, etc. and it is totally seamless: no difference between local and remote windows.

Numerous security issues in X Window System clients

Posted May 28, 2013 21:18 UTC (Tue) by raven667 (subscriber, #5198) [Link]

>> sure you can start an X term but there are many other ways to accomplish this functionality
>You do not understand. The magic of X remote is that it is always available.

I don't think you are disagreeing on this point. The point is to have remote display available, not to quibble about the details of how this is accomplished or to claim that X is the perfect design handed down from on high, never to be surpassed.

> totally seamless: no difference between local and remote windows.

I don't think that is generally true both in the underlying protocol and in the user experience. Differences in available fonts between the remote and local system is the most obvious thing I can think of but also anything depending on standard IPC such as DBus is also not seamless in the remote case and there is a great difference in how rendering is accomplished depending on whether shared memory or DRI are available.

Numerous security issues in X Window System clients

Posted May 24, 2013 16:33 UTC (Fri) by jg (subscriber, #17537) [Link]

All I can say is that in 1984-1988 it was a very, very different world. We're talking about > 25 years ago; many LWN readers weren't born then.

There weren't bad guys out there. I remember the Morris worm hitting when I was working on the X11 protocol bindings. And Kevin Mitnick had just reared his head a year or two before.

Some of the bugs I clearly wrote in that period, and others copied my mistakes (and clearly made more of their own). Others of the just couldn't happen in practice on machines of that era (say, 2 megabyte MicroVax's).

And X is still used widely remotely: just usually via SSH tunnels.

But the case you really worry about is different: remoting you application to an untrusted/untrustworthy X server, which may want to break into your machine/device. This is still a thing of desire (but SSH has highly discouraged this capability). So these problems, in practice, don't happen much.

I'm still happy to see the bugs get fixed, of course.

Numerous security issues in X Window System clients

Posted May 27, 2013 17:02 UTC (Mon) by FranTaylor (guest, #80190) [Link]

buffer overflow vulnerabilities have been documented since 1972

CheckedInt

Posted May 26, 2013 17:16 UTC (Sun) by bjacob (subscriber, #58566) [Link]

The pattern of bug described here, "integer overflows calculating memory needs for replies", sounds very familiar from working on browsers.

At Mozilla we've decided to implement a CheckedInt<T> templated class (where T is the integer type of your choice) that makes it easy to write safe (if somewhat slow) integer arithmetic.

It's a single self-contained header file:
http://hg.mozilla.org/mozilla-central/file/tip/mfbt/Check...

It has a unit test, too:
http://hg.mozilla.org/mozilla-central/file/tip/mfbt/tests...

It's been adopted by WebKit, too, but the above Mozilla link should be considered the canonical upstream (the WebKit file needs updating).
http://trac.webkit.org/browser/trunk/Source/WebCore/html/...

CheckedInt

Posted May 26, 2013 22:11 UTC (Sun) by foom (subscriber, #14868) [Link]

MS has a library, SafeInt, which looks pretty nice. http://safeint.codeplex.com/

CheckedInt

Posted May 27, 2013 0:16 UTC (Mon) by bjacob (subscriber, #58566) [Link]

We evaluated SafeInt, alongside other existing libraries, before deciding to do CheckedInt. Problems with SafeInt include:

1. SafeInt relies on throwing exceptions for integer arithmetic errors (at least if you want to use arithmetic operators), but many C++ projects, including most Mozilla projects, disable C++ exceptions.

2. SafeInt allows implicit conversion to plain integers. We decided not to allow that, as we wanted to have very simple semantics that would fit in the heads of code reviewers.

3. SafeInt is 6,918 lines, while CheckedInt is 818 lines (this includes whitespace and comments). SafeInt has separate code for each integer type, while CheckedInt has a single generic templated implementation. This makes it easier to trust CheckedInt.

4. SafeInt only supports __GNUC__ and MSVC. CheckedInt is portable. Besides explicit portability limitations that can be seen in #ifdefs, SafeInt relies on it being enough to support standard integer types e.g. int, long, long long; it is undefined whether that would be enough to support all stdint types such as int64_t.

CheckedInt

Posted May 30, 2013 4:05 UTC (Thu) by foom (subscriber, #14868) [Link]

Thanks for the detailed response, that was very useful information!

CheckedInt

Posted May 30, 2013 4:18 UTC (Thu) by rahulsundaram (subscriber, #21946) [Link]

There seems to be a renewed effort by the C++ standards committee to create a more expansive library after C++14 and I wonder is Mozilla involved in that effort in anyway?

CheckedInt

Posted Jun 3, 2013 11:47 UTC (Mon) by jwakely (subscriber, #60262) [Link]

No, I don't think they are involved.

Copyright © 2013, Eklektix, Inc.
Comments and public postings are copyrighted by their creators.
Linux is a registered trademark of Linus Torvalds