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

Issue 338538 link

Starred by 2 users

Issue metadata

Status: Fixed
Owner:
Closed: Jul 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Windows
Pri: 1
Type: Bug-Security



Sign in to add a comment

Security: Windows Sandbox Anonymous Kernel Object Unrestricted DACL

Reported by tyra...@gmail.com, Jan 27 2014

Issue description

VULNERABILITY DETAILS

The Windows sandbox relies heavily on duplicating various handles into the sandboxed processes from the broker to provide for things like IPC. Probably the most used is the SharedMemory class which wraps sharing of section objects up in a relatively platform independent manner. 

On Windows the SharedMemory class uses anonymous section objects (i.e. backed by swap and not named in the object namespace). This has the effect of assigning a default NULL DACL to the kernel object associated with this section. You can confirm this by looking up the section handle in something like Process Explorer or Process Hacker and checking its DACL. 

This has an interesting consequence when used to duplicate sections between processes, namely that the NULL DACL means there are effectively no permissions associated with the object (this includes bypassing integrity level checks). So a sandboxed process can use DuplicateHandle to increase the access mask of a section handle in their process. This might be most important if the broker code calls SharedMemory::ShareReadOnlyToProcess or similar assuming that it means the sandbox process cannot tamper with it, and only read it. 

The only place I've seen this being used, and not sure its for security necessarily is the sharing of user/Greasemonkey scripts. You might be able to cause issues with other renderer processes this way by changing the section to R/W and corrupting the data. You might also be able to resize the section to cause issues such as reducing it and then placing memory at that address.  

Note that this affects any sandboxed process including renderers. It probably will only work if the object was opened with write permissions first then mapped read only in the sandbox. Also note that it should affect any anonymous kernel object such as events and semaphores but they are probably uninteresting. 

VERSION
Chrome Version: 34.0.1806.0 (247121)
Operating System: Windows 7 SP1 x64

REPRODUCTION CASE

Just call DuplicateHandle in process on an anonymous section handle of your choosing. You should be able to increase permissions to include SECTION_ALL_ACCESS. 

 

Comment 1 by jsc...@chromium.org, Jan 27 2014

Cc: rvargas@chromium.org cpu@chromium.org wfh@chromium.org
This one is pretty cool. Carlos, Rick, thoughts?

Comment 2 by wfh@chromium.org, Jan 27 2014

Labels: OS-Windows Cr-Internals-Sandbox

Comment 3 by cpu@chromium.org, Jan 28 2014

Win7 only?

Comment 4 by tyra...@gmail.com, Jan 28 2014

I've done a basic test on Win8.1 x86, seems to work the same.
Yeah, a default security descriptor is not applied for unnamed sections... in fact, unnamed sections seem to be non-securable from what I see :(

Looks like either we remove the option to share "read-only" sections, or move to only use named sections, at least in that case.

BTW, too bad the unit test that was supposed to verify this behavior is broken :(

Comment 6 by wfh@chromium.org, Jan 28 2014

how about just adding a non-NULL token to the renderer duplicated object?  Sounds like this only affects NULL token.

Comment 7 Deleted

as far as I can see, unnamed sections are just not securable objects (I don't know if there is a complete, official list of unnamed object types that can be secured). And by "see" I mean I already tried that.

Comment 9 by tyra...@gmail.com, Jan 28 2014

You can find object types which can set a security descriptor based on calling NtQueryObject with the ObjectTypeInformation class. It returns a boolean SecurityRequired. If that isn't set only named objects get a security descriptor. 

From what I can tell that pretty much amounts to: Thread, Process, File, Key, WindowStation, Desktop, Token. Anything else if unnamed will not assign an SD.

Comment 10 by cpu@chromium.org, Jan 28 2014

Yeah, events are also broken this way. I wrote a test program (see attached).

This behavior was unknown to me, could it be that Windows is broken?
http://msdn.microsoft.com/en-us/library/windows/desktop/aa446598(v=vs.85).aspx

nulldacl.cpp
2.3 KB Download

Comment 11 by tyra...@gmail.com, Jan 28 2014

I believe it's by "design" in the sense that an anonymous kernel object is not shareable except through the duplication of handles as you can't open it in the via the NT object manager. I'm surprised there is _no_ way to set an SD though, that seems to be against the design principles of NT and its barely documented if at all. Might go and dig out my copy of Windows Internals :)
That not all unnamed objects are securable is documented:

http://msdn.microsoft.com/en-us/library/windows/desktop/aa379557(v=vs.85).aspx
Project Member

Comment 13 by ClusterFuzz, Jan 30 2014

Labels: Untriaged-1
Project Member

Comment 14 by ClusterFuzz, Feb 1 2014

Labels: Owner-Triage
Owner: jln@chromium.org
Status: Assigned
jln: Can you please take a look or find someone else to own it.

You are auto-assigned this issue since you are the top fixer for area label 'Cr-Internals-Sandbox'.

- Your friendly ClusterFuzz
Project Member

Comment 15 by ClusterFuzz, Feb 1 2014

Labels: Missing_Impact-2 Missing_Severity-2
Project Member

Comment 16 by ClusterFuzz, Feb 3 2014

Labels: -Missing_Impact-2 -Missing_Severity-2 Missing_Severity-3 Missing_Impact-3

Comment 17 by jln@chromium.org, Feb 5 2014

Cc: jln@chromium.org jsc...@chromium.org jyasskin@chromium.org ianbeer@chromium.org
Owner: wfh@chromium.org
This is somewhat similar to Linux with  issue 302724 . Also see the somewhat related issue 321818 (i.e., what is vulnerable if RO shared memory can be turned to RW?).

Will, can you own this?
Project Member

Comment 18 by ClusterFuzz, Feb 5 2014

Labels: -Missing_Severity-3 -Missing_Impact-3 Missing_Impact-4 Missing_Severity-4
Project Member

Comment 19 by ClusterFuzz, Feb 7 2014

Labels: -Missing_Impact-4 -Missing_Severity-4 Missing_Severity-5 Missing_Impact-5

Comment 20 by wfh@chromium.org, Feb 7 2014

Labels: -Untriaged-1 -Owner-Triage -Missing_Severity-5 -Missing_Impact-5 Security_Impact-Stable Security_Impact-Beta Security_Severity-Medium
Project Member

Comment 21 by ClusterFuzz, Feb 7 2014

Labels: Pri-1
Project Member

Comment 22 by ClusterFuzz, Feb 16 2014

Labels: Nag
wfh@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz
Labels: -Security_Severity-Medium Security_Severity-Low
Owner: ----
Status: Available
This really isn't medium. We need to fix it because in the future we will rely on enforcing things like read-only memory, but today we shouldn't be. So, bumping down to low-severity (just in case I missed something).
Project Member

Comment 24 by ClusterFuzz, Feb 16 2014

Labels: Owner-Triage
Owner: jln@chromium.org
Status: Assigned
jln: Can you please take a look or find someone else to own it.

You are auto-assigned this issue since you are the top fixer for area label 'Cr-Internals-Sandbox'.

- Your friendly ClusterFuzz
Cc: infe...@chromium.org
Owner: ----
Okay, time to remind Abhishek that the cf auto-assigning and nagging is still way too aggressive. It should not be doing that for low-severity bugs. I'll assign to myself just to shut it up for now, and keep it from making further erroneous assignments.

Comment 26 Deleted

Comment 27 by wfh@chromium.org, Jul 23 2014

Cc: forshaw@chromium.org
I've had the opportunity to look at this a little bit more, specifically around the handling of content/greasemonkey scripts. All renderers have a shared section passed in from the main process via UserScriptMaster. While this is marked as a read-only section as it's anonymous you can remap it to be writable inside a compromised renderer process. This section contains the pickled content scripts, if one is already registered you can modify it in the section and unless the list of scripts is changed by the master all new renderers will get the same copy. Depending on the content script modified it's possible to inject script into pages such as chrome://settings which in theory would allow a certain amount of privilege escalation. I don't know if this could be leveraged for a full sandbox escape but I guess it isn't impossible. 

For example on a fresh install of Canary on Windows (38.0.2102.0 64bit) the section contains a copy of saml_injected.js which is marked accessible for all urls. Changing this in memory to add 'alert(document.location);' and opening chrome://settings window demonstrates the injection was successful into chrome UI renderer content. 

I've not tested if this has an impact on other platforms. I believe Linux/OSX should correctly handle read-only permissions on sections. 
Cc: brettw@chromium.org
Project Member

Comment 30 by bugdroid1@chromium.org, Jul 24 2014

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dc84fcce71f37e6faeab9dce6ae2a65f1e12a5d2

commit dc84fcce71f37e6faeab9dce6ae2a65f1e12a5d2
Author: jschuh@chromium.org <jschuh@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Thu Jul 24 11:42:59 2014

Add a random name to anonymous shared memory sections

This prevents Windows from ignoring the DACL on unnamed sections.

BUG= 338538 
R=cpu@chromium.org,brettw@chromium.org

Review URL: https://codereview.chromium.org/412043002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@285195 0039d316-1c4b-4281-b951-d872f2087c98


Project Member

Comment 31 by bugdroid1@chromium.org, Jul 24 2014

------------------------------------------------------------------
r285195 | jschuh@chromium.org | 2014-07-24T11:42:59.987687Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/memory/shared_memory_win.cc?r1=285195&r2=285194&pathrev=285195

Add a random name to anonymous shared memory sections

This prevents Windows from ignoring the DACL on unnamed sections.

BUG= 338538 
R=cpu@chromium.org,brettw@chromium.org

Review URL: https://codereview.chromium.org/412043002
-----------------------------------------------------------------
Labels: -Security_Severity-Low -Nag -Owner-Triage Security_Severity-Medium Merge-Requested M-36
Status: Fixed
I'm going to assume the only shared unnamed objects we care about are sections, and count this one as fixed. The login renderer sideways escalation shouldn't be much additional risk, because a compromised renderer can already get at most of what's exposed. However, I'll bump it back up to medium and request a merge, since this should be a fairly risk-free merge.
Project Member

Comment 33 by ClusterFuzz, Jul 24 2014

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: -M-36 M-37
M-36 patch train, weird RM forgot to approve this :(:(, lets request merge for m37.
Justin: I guess you missed the comment on the review. Do you mind fixing the unit test?
Labels: -Merge-Requested Merge-Approved
merge approved for m37 branch 2062
Project Member

Comment 37 by bugdroid1@chromium.org, Aug 7 2014

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/6d6797eb60a6626fd48bd9fe92a6598de3a0ea8b

commit 6d6797eb60a6626fd48bd9fe92a6598de3a0ea8b
Author: jschuh@chromium.org <jschuh@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Thu Aug 07 22:07:43 2014

Add DACL and fix test for anonymous read-only memory

BUG= 338538 
R=rvargas@chromium.org,brettw@chromium.org
TBR=brettw@chromium.org

Review URL: https://codereview.chromium.org/444323005

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@288152 0039d316-1c4b-4281-b951-d872f2087c98


Project Member

Comment 38 by bugdroid1@chromium.org, Aug 7 2014

------------------------------------------------------------------
r288152 | jschuh@chromium.org | 2014-08-07T22:07:43.192544Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/memory/shared_memory_win.cc?r1=288152&r2=288151&pathrev=288152
   M http://src.chromium.org/viewvc/chrome/trunk/src/base/memory/shared_memory_unittest.cc?r1=288152&r2=288151&pathrev=288152

Add DACL and fix test for anonymous read-only memory

BUG= 338538 
R=rvargas@chromium.org,brettw@chromium.org
TBR=brettw@chromium.org

Review URL: https://codereview.chromium.org/444323005
-----------------------------------------------------------------
Labels: -Merge-Approved -M-37 M-38 Merge-NA
Labels: Release-0-M38
Labels: reward-topanel
Labels: -Security_Impact-Beta -reward-topanel reward-3000 reward-unpaid CVE-2014-3196
Congratulations tyranid - $3000 for this report.
Project Member

Comment 43 by ClusterFuzz, Oct 30 2014

Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Contacted tyranid re: payment.
Labels: -reward-unpaid reward-inprocess
Labels: -reward-inprocess
Processing via our e-payment system can take up to six weeks, but the reward should be on its way to you. Thanks again for your help!
If I have not misunderstood, instead of using anonymous shared memory you use named shared memory where name is randomly generated.
Whether the name of shared memory is randomly generated or not, it is named shared memory and as such it is possible to open it, read/write to it, it is even possible to do some code injection.
To list all shared memory of a process (chrome.exe) I have used ProcessExplorer.exe.
https://technet.microsoft.com/en-us/sysinternals/bb896653.aspx
In my test application, I've used GetNamedSecurityInfo and SetNamedSecurityInfo to create and set new security descriptor to an existing named shared memory. In this way it is possible to change the access rights to the shared memory. 
After that, I can use OpenFileMapping without any restrictions.
Re #47: Of course you can change the security descriptor or memory contents from outside the sandbox. The point of the sandbox is to protect the system from malicious code *inside* the sandbox. It's not possible to protect code inside the sandbox from malicious code running at privilege *outside* the sandbox, because by definition that code has full control over anything in the sandbox.
Project Member

Comment 49 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 50 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

Sign in to add a comment