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

Issue metadata

Status: Fixed
Owner:
Closed: Dec 2012
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: ----
Pri: 2
Type: Bug-Security



Sign in to add a comment

Security: Possible directory traversal vulnerability in ExtensionResource::GetFilePath().

Project Member Reported by tsepez@chromium.org, Nov 19 2012

Issue description

I "found" this via code inspection while looking at an unrelated bug, checking for places where folks might have existing code I could borrow to prevent directory traversal escapes.

So I didn't cobble up a test case.  Bad Bad Tommy.

Consider extension_resource.cc:66:

    for (std::vector<FilePath::StringType>::const_iterator
         i = components.begin(); i != components.end(); i++) {
      if (*i == FilePath::kParentDirectory) {
        depth--;
      } else {
        depth++;
      }
      if (depth < 0) {
        return FilePath();
      }
    }

This logic fails to account for "/." in path names, e.g. given something like ./.././.. we will be up two levels but will compute depth == 0.

Assigning to author. Please take a look. Hard to assign a severity because its unclear to me what other checks may be present (i.e. is "/." removed earlier).



 
Tom, did you just want to simply land a fix for this? I worry about these speculative bugs lying around in our security bug list indefinitely, because it's sometimes hard to motivate people to fix things without definite demonstration of impact.

Comment 2 by tsepez@chromium.org, Nov 26 2012

Cc: agl@chromium.org
Owner: tsepez@chromium.org
Sure.
Project Member

Comment 3 by bugdroid1@chromium.org, Nov 28 2012

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

------------------------------------------------------------------------
r169949 | tsepez@chromium.org | 2012-11-28T15:27:22.434778Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/extensions/extension_resource_unittest.cc?r1=169949&r2=169948&pathrev=169949
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/common/extensions/extension_resource.cc?r1=169949&r2=169948&pathrev=169949

Fix directory traversal in extension_resources.cc.  Adds test case, which is complicated by several other pre-existing checks.
BUG= 161836 

Review URL: https://chromiumcodereview.appspot.com/11308204
------------------------------------------------------------------------
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify Merge-Approved
Status: FixUnreleased
Labels: -SecSeverity-High -Merge-Approved SecSeverity-Low Merge-Merged Mstone-24 SecImpacts-Beta
Tom, I took SecSeverity down to Low because we haven't proven it's a problem?
Also, can extension resource loads be triggered from a compromised renderer? I guess yes since renderers can reference resources via chrome-extensions:// ?

Also, merged to M24
Project Member

Comment 6 by bugdroid1@chromium.org, Nov 30 2012

Labels: merge-merged-1312
The following revision refers to this bug:
    http://src.chromium.org/viewvc/chrome?view=rev&revision=170582

------------------------------------------------------------------------
r170582 | cevans@chromium.org | 2012-11-30T22:50:40.338989Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/chrome/common/extensions/extension_resource_unittest.cc?r1=170582&r2=170581&pathrev=170582
   M http://src.chromium.org/viewvc/chrome/branches/1312/src/chrome/common/extensions/extension_resource.cc?r1=170582&r2=170581&pathrev=170582

Merge 169949 - Fix directory traversal in extension_resources.cc.  Adds test case, which is complicated by several other pre-existing checks.
BUG= 161836 

Review URL: https://chromiumcodereview.appspot.com/11308204

TBR=tsepez@chromium.org
Review URL: https://codereview.chromium.org/11414274
------------------------------------------------------------------------
Labels: Release-0

Comment 8 by jsc...@chromium.org, Dec 20 2012

Status: Fixed
Labels: CVE-2013-0831
Project Member

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

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

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

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

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

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

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

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

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

Labels: -security_impact-beta
Project Member

Comment 16 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 17 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