Bug 47309 - Fix FileSystem path validation order to normalize '..' and '.' before restriction checks
Summary: Fix FileSystem path validation order to normalize '..' and '.' before restric...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-10-06 15:33 PDT by Kinuko Yasuda
Modified: 2010-10-08 01:39 PDT (History)
2 users (show)

See Also:


Attachments
Patch (8.45 KB, patch)
2010-10-07 16:32 PDT, Kinuko Yasuda
levin: review+
levin: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kinuko Yasuda 2010-10-06 15:33:23 PDT
Fix FileSystem path validation order to normalize '..' and '.' before restriction checks.

Currently we're rejecting any paths that contain '.' or '..' as invalid paths (since they end with '.', which is restricted in the spec).
Normalize the path first and then do the name validation.
Comment 1 Kinuko Yasuda 2010-10-07 16:32:38 PDT
Created attachment 70168 [details]
Patch
Comment 2 Eric U. 2010-10-07 19:26:42 PDT
Comment on attachment 70168 [details]
Patch

LGTM
Comment 3 David Levin 2010-10-07 23:56:59 PDT
Comment on attachment 70168 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=70168&action=review

> LayoutTests/fast/filesystem/resources/op-get-entry.js:60
> +            function(helper) { helper.getFile('/a', '/a/../../b/./c/../../../../../e.txt', {create:true}); },

Consider adding a test with "..." to check that it is invalid (unless you already have one).

> WebCore/fileapi/DOMFileSystemBase.cpp:145
> +    if (DOMFilePath::isAbsolute(path))

How about: 
if (!DOMFilePath::isAbsolute(path))
   path = DOMFilePath::append(base->fullPath(), path);
String absolutePath = DOMFilePath::removeExtraParentReferences(path);

In fact make this all into a function b/c there is so much common code in these two places.

Feel free to change names but this is what I'm thinking:

bool DOMFileSystemBase::domPathToPlatformPath(String domPath, String& platformPath)
{
    if (!DOMFilePath::isAbsolute(path))
        domPath = DOMFilePath::append(base->fullPath(), domPath);
    String absolutePath = DOMFilePath::removeExtraParentReferences(path);

    if (!DOMFilePath::isValidPath(absolutePath))
        return false;
    platformPath = m_asyncFileSystem->virtualToPlatformPath(absolutePath);
    return true;
}

String platformPath;
if (!domPathToPlatformPath(path, platformPath))
    return false;
Comment 4 Kinuko Yasuda 2010-10-08 01:39:05 PDT
Committed r69382: <http://trac.webkit.org/changeset/69382>