New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 143859 link

Starred by 3 users

Issue metadata

Status: Verified
Owner:
Closed: Dec 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Linux , Chrome
Pri: 2
Type: Bug-Security



Sign in to add a comment

Security: World-writable shared memory segments for X/Linux UI

Project Member Reported by palmer@google.com, Aug 21 2012

Issue description

ui/surface/transport_dib_linux.cc and ui/base/x/x11_util.cc create shared memory segments that have mode 0666. ui/surface/transport_dib_linux.cc has the comment:

TransportDIB* TransportDIB::Create(size_t size, uint32 sequence_num) {
  // We use a mode of 0666 since the X server won't attach to memory which is
  // 0600 since it can't know if it (as a root process) is being asked to map
  // someone else's private shared memory region.
  const int shmkey = shmget(IPC_PRIVATE, size, 0666);

but that comment does not appear to be true anymore. (I changed the mode the 0600 and added a DLOG(WARNING) telling me if shmkey worked, and it did. And chrome appears to run normally.)

ccing agl since he wrote the shmget calls in both files. I've got a trivial patch that I'll post momentarily.
 
Are you sure Chrome didn't fall back to non-SHM X?

You want to check the XShmAttach calls actually work:

https://code.google.com/p/chromium/source/search?q=xshmattach&origq=xshmattach&btnG=Search+Trunk

The x11_util.cc calls are interesting (and one is actually a "probe" to see if XShm works). backing_store_gtk.cc also looks relevant.

Comment 2 by palmer@google.com, Aug 29 2012

Adding lots of DCHECKS causes no crashes (there is also a NOTREACHED in x11_util.cc that I do not hit), and DLOG(WARNING) about the results of XShmAttach and its callers in x11_util.cc prints happiness. I can't seem to get DLOGs in XShmAttach callers in backing_store_gtk.cc to hit — I'm not exercising that code for some reason.

Does anyone know of a good way to hit all this code? Are there unit tests?

Anyway I am sure XShmAttach *is* working in the places I am hitting it, even with mode 0600. So that seems like a good sign.

Comment 3 by agl@chromium.org, Aug 29 2012

Ok. If it seems to work and it still seems to work on gHardy then we can give it a shot.

Comment 4 by palmer@chromium.org, Aug 29 2012

gHardy?

Comment 5 by agl@chromium.org, Aug 29 2012

Hardy = Ubuntu 8.04 LTS, sorry.
I don't think Chrome would even install on such ancient machines??

When 8.04 LTS went out of support, we lept on the opportunity to upgrade our build infrastructure to 10.04 LTS -- giving us the -fPIE part of ASLR finally. I think the libstdc++ etc. versions we now use wouldn't work on 8.04 LTS?

Comment 7 by agl@chromium.org, Aug 29 2012

Ah, then I guess that Lucid (Ubuntu 10.04 LTS) is the oldest that we support now. (Although X certainly *does* run as root on Lucid)

Comment 8 by palmer@chromium.org, Aug 29 2012

gLucid is what I have tested it on so far. Going to try my gPrecise next.

Comment 9 by palmer@chromium.org, Aug 29 2012

Works on gPrecise.
Note that both gLucid and gPrecise do run X as root. gLucid:

root      1748  1.2  0.1 148836 49112 tty7     Ss+  Aug28  19:34 /usr/bin/X :0 -nr -verbose -auth /var/run/gdm/auth-for-gdm-X8wxzs/database -nolisten tcp vt7

gPrecise:

root      1952  0.8  0.0 160872 53440 tty7     Rs+  13:44   1:42 /usr/bin/X :0 -auth /var/run/lightdm/root/:0 -nolisten tcp vt7 -novtswitch -background none

cevans and jorgelo say they think there is no Linux distro (at least that we support) that has a non-root X. My own knowing-what-Linux-distros-are-up-to days are behind me, so I'm going to believe them. agl, do you know differently?

Comment 11 by jln@chromium.org, Sep 10 2012

Cc: markus@chromium.org jln@chromium.org
Labels: -Mstone-22 -SecSeverity-Medium Mstone-24 SecSeverity-Low
@palmer: looks like you have the LGTMs to land this. Since we just branched M23 off, this is a great time to land this for early M24 testing.

Comment 14 by palmer@google.com, Sep 21 2012

Although I could hit the CQ button now, I don't feel good doing so while I am on vacation, since I won't be around to watch it succeed or fail to land. If you or someone else would like to do so, that's fine; otherwise I'll take care of it on Monday.
Project Member

Comment 15 by bugdroid1@chromium.org, Sep 24 2012

The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=158289

------------------------------------------------------------------------
r158289 | palmer@chromium.org | 2012-09-24T16:26:46.224812Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/base/x/x11_util.cc?r1=158289&r2=158288&pathrev=158289
   M http://src.chromium.org/viewvc/chrome/trunk/src/ui/surface/transport_dib_linux.cc?r1=158289&r2=158288&pathrev=158289
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/backing_store_gtk.cc?r1=158289&r2=158288&pathrev=158289

Make shared memory segments writable only by their rightful owners.

BUG= 143859 
TEST=Chrome's UI still works on Linux and Chrome OS
Review URL: https://chromiumcodereview.appspot.com/10854242
------------------------------------------------------------------------
Status: Fixed
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Status: FixUnreleased
Labels: Release-0
Status: Fixed
Labels: CVE-2013-0838
Labels: VerifyIn-26
Project Member

Comment 22 by bugdroid1@chromium.org, Mar 10 2013

Labels: -Type-Security -Area-Internals -Mstone-24 -SecImpacts-Stable -SecImpacts-Beta -SecSeverity-Low Security-Severity-Low Security-Impact-Stable Security-Impact-Beta Cr-Internals M-24 Type-Bug-Security
Labels: -Restrict-View-SecurityNotify
Project Member

Comment 24 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Severity-Low Security_Severity-Low
Project Member

Comment 25 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member

Comment 26 by bugdroid1@chromium.org, Mar 21 2013

Labels: -Security-Impact-Beta Security_Impact-Beta
Labels: VerifyIn-27
Status: Verified
Project Member

Comment 29 by sheriffbot@chromium.org, Jun 14 2016

Labels: -security_impact-beta
Project Member

Comment 30 by sheriffbot@chromium.org, Oct 1 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 31 by sheriffbot@chromium.org, Oct 2 2016

This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Labels: allpublic
Labels: CVE_description-submitted
Project Member

Comment 34 by sheriffbot@chromium.org, Jul 28

Labels: -Pri-1 Pri-2

Sign in to add a comment