Bug 47309

Summary: Fix FileSystem path validation order to normalize '..' and '.' before restriction checks
Product: WebKit Reporter: Kinuko Yasuda <kinuko>
Component: WebCore Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: ericu, levin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: OS X 10.5   
Attachments:
Description Flags
Patch levin: review+, levin: commit-queue-

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>