Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Issue 162114 Security: Renderer sandbox bypass by crafting LevelDB database in "profile/File System/"
Starred by 1 user Project Member Reported by aedla@chromium.org, Nov 20 2012 Back to list
Status: Fixed
Owner:
Closed: Dec 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 0
Type: Bug-Security



Sign in to add a comment
ObfuscatedFileUtil keeps its LevelDB databases in "profile/File System/". The databases store paths of files/folders created by the File System API.

Renderer can modify those databases because it has write permission to "profile/File System" as granted by RenderProcessHostImpl constructor. So the renderer can point the paths to any file in the file system using path traversal: ../../../../../../../etc/passwd. It can then use the File System API to read/write system files.

I created a PoC that creates a directory under user's home in linux using the origin database. Files can be created similarly using the directory database.

VERSION
Chrome Version: 25.0.1331.0 (Developer Build 168824)
Operating System: Ubuntu 12.04 x64

REPRODUCTION CASE

Apply the attached patch to content/renderer/render_thread_impl.cc. I have only tested this in linux.

 
renderer.patch
4.5 KB View Download
Comment 1 by tsepez@chromium.org, Nov 20 2012
Assuming the ChildProcessSecurityPolicy bug is fixed, is this still an issue?
Comment 2 by aedla@chromium.org, Nov 20 2012
@tsepez: It is. Browser doesn't check the policy anymore for paths read from the database.

Comment 3 by tsepez@chromium.org, Nov 20 2012
Owner: kinuko@chromium.org
Status: Assigned
@kinuko seems like a reasonable assignee based upon change history.

Two ways of fixing this:
1) Move the metadata out of profile/File System/ into its own directory where the renderer has not privileges.
2) Sanity check that the returned path as read from the database is under the expected directory.

I'd suggest both.
Comment 4 by tsepez@chromium.org, Nov 20 2012
Cc: ericu@chromium.org
Comment 5 by ericu@chromium.org, Nov 20 2012
I'm out until Monday, but would be happy to grab this one as soon as I'm in.
Comment 6 by jln@chromium.org, Nov 21 2012
Cc: ajwong@chromium.org
If someone could jump on this, it would be awesome. We'll be targeting a fix to an "asap" M23 patch, so the sooner we can get bake time on trunk the better.

I concur with Tom's #c3 that we probably want both possible defenses listed.
Comment 8 by kinuko@chromium.org, Nov 26 2012
I'm taking a look.
Comment 9 by kinuko@chromium.org, Nov 26 2012
I might be wrong but as for the renderer privileges I'm feeling that we shouldn't have granted any direct permission to the hidden sandboxed area for this API from the beginning. I'm creating a proposal patch to do this and 2) in #c3.
Labels: Audit-IPC
Project Member Comment 11 by bugdroid1@chromium.org, Nov 29 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=170159

------------------------------------------------------------------------
r170159 | kinuko@chromium.org | 2012-11-29T10:38:52.769878Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/worker_host/worker_process_host.cc?r1=170159&r2=170158&pathrev=170159
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/fileapi/fileapi_message_filter.cc?r1=170159&r2=170158&pathrev=170159
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/fileapi/file_system_directory_database.cc?r1=170159&r2=170158&pathrev=170159
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/renderer_host/render_process_host_impl.cc?r1=170159&r2=170158&pathrev=170159
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/fileapi/file_system_directory_database.h?r1=170159&r2=170158&pathrev=170159
   M http://src.chromium.org/viewvc/chrome/trunk/src/webkit/fileapi/sandbox_mount_point_provider.cc?r1=170159&r2=170158&pathrev=170159

Do not give blanket permission to the Sandboxed FileSystem directory and verify resolved paths

- Avoid giving unconditional blanket permission to the entire sandbox FileSystem data directory
- Perform the sanity check that the resolved path from the database is under the expected directory

BUG= 162114 
TEST=manual

Review URL: https://codereview.chromium.org/11308194
------------------------------------------------------------------------
Cc: tzik@chromium.org
Project Member Comment 13 by bugdroid1@chromium.org, Nov 29 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=170181

------------------------------------------------------------------------
r170181 | kinuko@chromium.org | 2012-11-29T14:39:53.610926Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/fileapi/fileapi_message_filter.cc?r1=170181&r2=170180&pathrev=170181
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/fileapi/fileapi_message_filter.h?r1=170181&r2=170180&pathrev=170181

File permission fix: now we selectively grant read permission for Sandboxed files

We also need to check the read permission and call GrantReadFile() for
sandboxed files for CreateSnapshotFile().

BUG= 162114 
TEST=manual

Review URL: https://codereview.chromium.org/11280231
------------------------------------------------------------------------
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify Merge-Approved
Status: FixUnreleased
Labels: Mstone-23
@kinuko: thanks for the fix!!! :D
Seems like a larger / possibly risky change? Should we wait for M24 to merge this fix, or would it be safe for M23 if it canaries OK over the weekend?
@scarybeasts I might want to wait a few more days to see no one yells on canaries, but assuming that this could cause a serious security issue after that I guess we'd be ok to merge this to M23. (Fingers crossed)
Labels: -Mstone-23 Mstone-24
@kinuko: we can wait until M24, which isn't too far out. Although merging to M23 and crossing fingers sounds like a fun ride, I doubt Karen would be happy with me :)
Project Member Comment 18 by bugdroid1@chromium.org, Dec 4 2012
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=171068

------------------------------------------------------------------------
r171068 | cevans@chromium.org | 2012-12-04T22:42:52.954494Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/content/browser/fileapi/fileapi_message_filter.h?r1=171068&r2=171067&pathrev=171068
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/content/browser/fileapi/fileapi_message_filter.cc?r1=171068&r2=171067&pathrev=171068

Merge 170181
BUG= 162114 
Review URL: https://codereview.chromium.org/11414315
------------------------------------------------------------------------
Labels: Release-0
Status: Fixed
Labels: CVE-2013-0829
Project Member Comment 23 by bugdroid1@chromium.org, Mar 10 2013
Labels: -Type-Security -Area-Internals -SecSeverity-High -SecImpacts-Stable -SecImpacts-Beta -Mstone-24 Security-Impact-Stable Security-Impact-Beta Cr-Internals M-24 Security-Severity-High Type-Bug-Security
Labels: -Restrict-View-SecurityNotify
Project Member Comment 25 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Severity-High Security_Severity-High
Project Member Comment 26 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-Stable Security_Impact-Stable
Project Member Comment 27 by bugdroid1@chromium.org, Mar 21 2013
Labels: -Security-Impact-Beta Security_Impact-Beta
Project Member Comment 28 by sheriffbot@chromium.org, Jun 14 2016
Labels: -security_impact-beta
Project Member Comment 29 by sheriffbot@chromium.org, Oct 1
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 30 by sheriffbot@chromium.org, Oct 2
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
Sign in to add a comment