typo in mod() macro leads to 3rd-party controllable Xorg crash/exploit

Bug #551193 reported by Kees Cook on 2010-03-29
258
This bug affects 1 person
Affects Status Importance Assigned to Milestone
X.Org X server
Fix Released
High
xorg-server (Ubuntu)
High
Unassigned
Hardy
Undecided
Unassigned
Jaunty
Undecided
Unassigned
Karmic
Undecided
Unassigned
Lucid
High
Unassigned

Description of problem:
When attempting to load http://www.buffalonews.com/entertainment/moviestv/index.html Xorg crashes:

#0 0x0020161f in fbCopyAreammx (pSrc=0x8cdea90, pDst=0x8cb9620, src_x=0, src_y=3846, dst_x=18, dst_y=3871, width=920, height=60788) at fbmmx.c:2240
#1 0x00201776 in fbCompositeCopyAreammx (op=1 '\001', pSrc=0x8cf5030, pMask=0x0, pDst=0x8c304f0, xSrc=0, ySrc=3846, xMask=0, yMask=0, xDst=18, yDst=3871, width=920, height=64598) at fbmmx.c:2303
#2 0x001f24fb in fbComposite (op=1 '\001', pSrc=0x8cf5030, pMask=0x0, pDst=0x8c304f0, xSrc=0, ySrc=994, xMask=0, yMask=0, xDst=18, yDst=3871, width=920, height=178) at fbpict.c:1299
#3 0x00247eeb in XAAComposite (op=1 '\001', pSrc=0x8cf5030, pMask=0x0, pDst=0x8c304f0, xSrc=0, ySrc=-926, xMask=0, yMask=0, xDst=18, yDst=789, width=920, height=178) at xaaPict.c:536
#4 0x0070c8d8 in i830_xaa_composite (op=1 '\001', pSrc=0x8cf5030, pMask=0x0, pDst=0x8c304f0, xSrc=0, ySrc=-926, xMask=0, yMask=0, xDst=18, yDst=789, width=920, height=178) at i830_xaa.c:873
#5 0x0815e026 in cwComposite (op=1 '\001', pSrcPicture=0x8cf5030, pMskPicture=0x0, pDstPicture=0x8c304f0, xSrc=0, ySrc=-926, xMsk=0, yMsk=0, xDst=18, yDst=789, width=920, height=178) at cw_render.c:275
#6 0x0815a996 in damageComposite (op=1 '\001', pSrc=0x8cf5030, pMask=0x0, pDst=0x8c304f0, xSrc=0, ySrc=-926, xMask=0, yMask=0, xDst=18, yDst=789, width=920, height=178) at damage.c:541
#7 0x08147b23 in CompositePicture (op=1 '\001', pSrc=0x8cf5030, pMask=0x0, pDst=0x8c304f0, xSrc=0, ySrc=-926, xMask=0, yMask=0, xDst=18, yDst=789, width=920, height=178) at picture.c:1789
#8 0x0814d95f in ProcRenderComposite (client=0x8c6c5c8) at render.c:758
#9 0x0814acd5 in ProcRenderDispatch (client=0xafa4f000) at render.c:2005
#10 0x0808815a in Dispatch () at dispatch.c:459
#11 0x0806fab5 in main (argc=10, argv=0xbfefe174, envp=Cannot access memory at address 0xafa4f008

This also hoses my virtual terminals (ctrl+alt+F1 gives me a white screen) and I have to reboot to get them back.

Version-Release number of selected component (if applicable):
xorg-x11-server-Xorg-1.1.1-48.52.el5

How reproducible:
always

Steps to Reproduce:
1. Grab the latest Firefox 3.5:
http://ftp.mozilla.org/pub/mozilla.org/firefox/nightly/latest-mozilla-1.9.1/firefox-3.5b4pre.en-US.linux-i686.tar.bz2
2. install it (bzip2 -dc firefox*bz2 | tar -x)
3. Run it (cd firefox; ./firefox)
4. Visit http://www.buffalonews.com/entertainment/moviestv/index.html

==> crash

Thanks for the bug report. We have reviewed the information you have provided above, and there is some additional information we require that will be helpful in our diagnosis of this issue.

Please attach your X server config file (/etc/X11/xorg.conf, if available) and X server log file (/var/log/Xorg.*.log) to the bug report as individual uncompressed file attachments using the bugzilla file attachment link below.

We will review this issue again once you've had a chance to attach this information.

Thanks in advance.

Created attachment 341765
X log

Created attachment 341766
xorg.conf

Created attachment 341775
Xorg-crashing html testcase

X crasher HTML actually doesn't show anything on this RHEL5's firefox-3.0.10-1.el5 and neither the real page crashes (I have flash-plugin installed and Javascript fully on).

Moreover, I don't see anything suspicious in the Xorg.0.log.

Are you able to reproduce this issue with firefox running in the safe mode (i.e. run firefox -safe-mode from the command line)?

Possibly are you able to reproduce this with the upstream binary from mozilla.com?

Thank you very much for filing this bug report and your cooperation with its solving.

> X crasher HTML actually doesn't show anything on this RHEL5's
> firefox-3.0.10-1.el5 and neither the real page crashes

Yes. Only firefox 3.5 (currently in beta) causes a crash.

> I don't see anything suspicious in the Xorg.0.log.

Right. Note that I have 'Option "NoTrapSignals"' in my xorg.conf, which prevents the seg fault notice and mangled stack from showing in the log file (I added it so I could use gdb to get the more useful stack in comment 0).

> Possibly are you able to reproduce this with the upstream binary from
> mozilla.com?

Yes. upstream binaries (see steps to reproduce in comment 0) and builds compiled on RHEL5 both cause the crash.

Does adding: Option "XaaNoOffscreenPixmaps" to the device section of your xorg.conf work around the issue?

Just for the record, I have encountered this problem with the latest version
(I think) of the server, xorg-x11-server-Xorg-1.1.1-48.67.el5, with the page:
http://www.thirdage.com/health-wellness, and the workaround from comment #7,
above has worked for me too.

Created attachment 402099
Possible patch

Looks like the problem occurs because the size passed in fbCompose() to the mmx function are negative (either h_this or w_this is < 0).

The mmx equivalent in fbmmx.c seem to expect a CARD16 for width / height so that might explain the problem.

This patch seems to address the issue at least in my reproducer, but I am not sure of its possible side effects.

Comment on attachment 402099
Possible patch

Patch does not fix all cases, marking obsolete.

Created attachment 403292
Proposed patch

The problem comes from the macro mod() used in computation.

The code in fbComposite() from fbpict.c reads like this:

    if (srcRepeat)
    {
        y_src = mod (y_src - pSrc->pDrawable->y, pSrc->pDrawable->height);
        if (h_this > pSrc->pDrawable->height - y_src)
            h_this = pSrc->pDrawable->height - y_src;
        y_src += pSrc->pDrawable->y;
    }

While inspecting the values, we see that initially, y_src=871, pSrc->pDrawable->y=1024, pSrc->pDrawable->height=500

After computation of mod() y_src=895 (which is wrong) so that h_this = pSrc->pDrawable->height - y_src = -395

Passing a negative value to a CARD16 in mmx function will cause the crash. But the real problem is that the value returned by mod() is actually greater than the values passed which is not possible, so there should be no way that y_src is greater than pSrc->pDrawable->height and therefore h_this should/could not be negative.

mod() is defined as follow (earlier in that code):

# define mod(a,b) ((b) == 1 ? 0 : (a) >= 0 ? (a) % (b) : (b) - (-a) % (b))

Problem is that (-a) gets expanded as "-871 - 1024" (and *not* "- (871 - 1024)" as expected) and therefore "(b) - (-a) % (b)" = 500 - (-871 - 1024) = 895

TI think the following would be more appropriate:

# define mod(a,b) ((b) == 1 ? 0 : (a) >= 0 ? (a) % (b) : (b) - (-(a)) % (b))

That seems to fix the crash and produces the correct output.

Kees Cook (kees) wrote :

It's not clear how affected Ubuntu is due to the deprecation of XAA. Is fbComposite() used in other call paths?

security vulnerability: no → yes
Bryce Harrington (bryce) on 2010-03-29
Changed in xorg-server (Ubuntu):
assignee: nobody → Bryce Harrington (bryceharrington)
importance: Undecided → High
milestone: none → ubuntu-10.04-beta-2
status: New → Fix Committed
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package xorg-server - 2:1.7.6-1ubuntu2

---------------
xorg-server (2:1.7.6-1ubuntu2) lucid; urgency=low

  * Add 112_xaa-fbcomposite-fix-negative-size.patch: Prevent 3rd-party
    controllable Xorg crash/exploit when XAA and compositing is in use.
    Be more careful in sign-changes with mod().
    (LP: #551193)
 -- Bryce Harrington <email address hidden> Mon, 29 Mar 2010 13:37:06 -0700

Changed in xorg-server (Ubuntu):
status: Fix Committed → Fix Released

Return value seems bit odd for a being a negative multiple of positive b. For positive a, return values range from 0 to b-1, while for negative a, mod returns 1 to b.

True, but the problem the patch is meant to address primarily is the X server crash induced by a wrong macro expansion (rather than changing the macro itself).

For comparison, Pixman uses that definition:

  #define MOD(a, b) ((a) < 0 ? ((b) - ((-(a) - 1) % (b))) - 1 : (a) % (b))

in http://cgit.freedesktop.org/pixman/tree/pixman/pixman-private.h

And uses of that macro for example in walk_region_internal() from http://cgit.freedesktop.org/pixman/tree/pixman/pixman.c which is quite similar to the implementation of the "old" fbComposite()

Devel ack, patch is obviously correct.

*** Bug 570089 has been marked as a duplicate of this bug. ***

An advisory has been issued which should help the problem
described in this bug report. This report is therefore being
closed with a resolution of ERRATA. For more information
on therefore solution and/or where to find the updated files,
please follow the link below. You may reopen this bug report
if the solution does not work for you.

http://rhn.redhat.com/errata/RHSA-2010-0382.html

Kees Cook (kees) wrote :

CVE-2010-1166

Kees Cook (kees) on 2010-05-06
Changed in xorg-server (Ubuntu Lucid):
status: New → Fix Released
Changed in xorg-server (Ubuntu):
milestone: ubuntu-10.04-beta-2 → none
Changed in xorg-server (Ubuntu Lucid):
importance: Undecided → High
Bryce Harrington (bryce) on 2010-05-06
Changed in xorg-server (Ubuntu):
assignee: Bryce Harrington (bryceharrington) → nobody
Launchpad Janitor (janitor) wrote :

This bug was fixed in the package xorg-server - 2:1.6.4-2ubuntu4.3

---------------
xorg-server (2:1.6.4-2ubuntu4.3) karmic-security; urgency=low

  * SECURITY UPDATE: incorrect mod() macro could result in crashes
    caused by remote attackers (LP: #551193).
    - Added debian/patches/300_xaa-fbcomposite-fix-negative-size.patch
    - CVE-2010-1166
 -- Kees Cook <email address hidden> Thu, 06 May 2010 13:16:02 -0700

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package xorg-server - 2:1.6.0-0ubuntu14.2

---------------
xorg-server (2:1.6.0-0ubuntu14.2) jaunty-security; urgency=low

  * SECURITY UPDATE: incorrect mod() macro could result in crashes
    caused by remote attackers (LP: #551193).
    - Added debian/patches/xaa-fbcomposite-fix-negative-size.patch
    - CVE-2010-1166
  * SECURITY UPDATE: xvfb MCOOKIE value could be hijacked due to
    visiblity on the command-line,
    - Updated debian/local/xvfb-run from Debian upstream:
      http://git.debian.org/?p=pkg-xorg/xserver/xorg-server.git;a=commitdiff;h=ecf09e571198ee16256a5efd1c23fd286a4f2249;hp=cbccf51785b500f51dc974ed05f5512181d4c51f
    - CVE-2009-1573
 -- Kees Cook <email address hidden> Thu, 06 May 2010 13:23:52 -0700

Launchpad Janitor (janitor) wrote :

This bug was fixed in the package xorg-server - 2:1.4.1~git20080131-1ubuntu9.3

---------------
xorg-server (2:1.4.1~git20080131-1ubuntu9.3) hardy-security; urgency=low

  * SECURITY UPDATE: incorrect mod() macro could result in crashes
    caused by remote attackers (LP: #551193).
    - Added debian/patches/xaa-fbcomposite-fix-negative-size.patch
    - CVE-2010-1166
  * SECURITY UPDATE: xvfb MCOOKIE value could be hijacked due to
    visiblity on the command-line,
    - Updated debian/local/xvfb-run from Debian upstream:
      http://git.debian.org/?p=pkg-xorg/xserver/xorg-server.git;a=commitdiff;h=ecf09e571198ee16256a5efd1c23fd286a4f2249;hp=cbccf51785b500f51dc974ed05f5512181d4c51f
    - CVE-2009-1573
 -- Kees Cook <email address hidden> Thu, 06 May 2010 13:26:51 -0700

Changed in xorg-server (Ubuntu Hardy):
status: New → Fix Released
Changed in xorg-server (Ubuntu Jaunty):
status: New → Fix Released
Changed in xorg-server (Ubuntu Karmic):
status: New → Fix Released
Changed in xorg-server:
importance: Unknown → High
status: Unknown → Fix Released
To post a comment you must log in.
This report contains Public Security information  Edit
Everyone can see this security related information.

Other bug subscribers

Remote bug watches

Bug watches keep track of this bug in other bug trackers.