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-

Kinuko Yasuda
Reported 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.
Attachments
Patch (8.45 KB, patch)
2010-10-07 16:32 PDT, Kinuko Yasuda
levin: review+
levin: commit-queue-
Kinuko Yasuda
Comment 1 2010-10-07 16:32:38 PDT
Eric U.
Comment 2 2010-10-07 19:26:42 PDT
Comment on attachment 70168 [details] Patch LGTM
David Levin
Comment 3 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;
Kinuko Yasuda
Comment 4 2010-10-08 01:39:05 PDT
Note You need to log in before you can comment on or make changes to this bug.