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:

Version:

528+ (Nightly build)

Hardware:

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-	Details Formatted Diff Diff
View All Add attachment proposed patch, testcase, etc.

Kinuko Yasuda

Comment 1 2010-10-07 16:32:38 PDT

Created attachment 70168 [details] Patch

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

Committed r69382: <http://trac.webkit.org/changeset/69382>

Note You need to log in before you can comment on or make changes to this bug.