diff --git a/dom/devicestorage/DeviceStorage.h b/dom/devicestorage/DeviceStorage.h index 4bc102e3c7e2..d36ad55d93b1 100644 --- a/dom/devicestorage/DeviceStorage.h +++ b/dom/devicestorage/DeviceStorage.h @@ -101,8 +101,9 @@ public: // we want to make sure that the names of file can't reach // outside of the type of storage the user asked for. - bool IsSafePath(); - bool IsSafePath(const nsAString& aPath); + bool IsSafePath() const; + bool ValidateAndSplitPath(const nsAString& aPath, + nsTArray* aParts = nullptr) const; void Dump(const char* label); @@ -137,7 +138,6 @@ public: private: ~DeviceStorageFile() {} void Init(); - void NormalizeFilePath(); void AppendRelativePath(const nsAString& aPath); void AccumDirectoryUsage(nsIFile* aFile, uint64_t* aPicturesSoFar, diff --git a/dom/devicestorage/nsDeviceStorage.cpp b/dom/devicestorage/nsDeviceStorage.cpp index f77eb806d40b..fdc432fef3a2 100644 --- a/dom/devicestorage/nsDeviceStorage.cpp +++ b/dom/devicestorage/nsDeviceStorage.cpp @@ -30,6 +30,7 @@ #include "nsArrayUtils.h" #include "nsAutoPtr.h" +#include "nsCharSeparatedTokenizer.h" #include "nsGlobalWindow.h" #include "nsServiceManagerUtils.h" #include "nsIFile.h" @@ -76,10 +77,35 @@ using namespace mozilla::dom; using namespace mozilla::dom::devicestorage; using namespace mozilla::ipc; -namespace mozilla { +namespace mozilla +{ MOZ_TYPE_SPECIFIC_SCOPED_POINTER_TEMPLATE(ScopedPRFileDesc, PRFileDesc, PR_Close); } // namespace mozilla +namespace { + +void +NormalizeFilePath(nsAString& aPath) +{ +#if defined(XP_WIN) + char16_t* cur = aPath.BeginWriting(); + char16_t* end = aPath.EndWriting(); + for (; cur < end; ++cur) { + if (char16_t('\\') == *cur) { + *cur = FILESYSTEM_DOM_PATH_SEPARATOR_CHAR; + } + } +#endif +} + +bool +TokenizerIgnoreNothing(char16_t /* aChar */) +{ + return false; +} + +} // anonymous namespace + StaticAutoPtr DeviceStorageUsedSpaceCache::sDeviceStorageUsedSpaceCache; @@ -510,7 +536,8 @@ DeviceStorageFile::DeviceStorageFile(const nsAString& aStorageType, if (!mPath.EqualsLiteral("")) { AppendRelativePath(mPath); } - NormalizeFilePath(); + + NormalizeFilePath(mPath); } DeviceStorageFile::DeviceStorageFile(const nsAString& aStorageType, @@ -525,7 +552,7 @@ DeviceStorageFile::DeviceStorageFile(const nsAString& aStorageType, { Init(); AppendRelativePath(aPath); - NormalizeFilePath(); + NormalizeFilePath(mPath); } DeviceStorageFile::DeviceStorageFile(const nsAString& aStorageType, @@ -734,7 +761,7 @@ DeviceStorageFile::CreateUnique(const nsAString& aStorageType, void DeviceStorageFile::SetPath(const nsAString& aPath) { mPath.Assign(aPath); - NormalizeFilePath(); + NormalizeFilePath(mPath); } void @@ -745,13 +772,14 @@ DeviceStorageFile::SetEditable(bool aEditable) { // we want to make sure that the names of file can't reach // outside of the type of storage the user asked for. bool -DeviceStorageFile::IsSafePath() +DeviceStorageFile::IsSafePath() const { - return IsSafePath(mRootDir) && IsSafePath(mPath); + return ValidateAndSplitPath(mRootDir) && ValidateAndSplitPath(mPath); } bool -DeviceStorageFile::IsSafePath(const nsAString& aPath) +DeviceStorageFile::ValidateAndSplitPath(const nsAString& aPath, + nsTArray* aParts) const { nsAString::const_iterator start, end; aPath.BeginReading(start); @@ -764,33 +792,43 @@ DeviceStorageFile::IsSafePath(const nsAString& aPath) StringBeginsWith(aPath, tildeSlash)) { NS_WARNING("Path name starts with tilde!"); return false; - } - // split on /. if any token is "", ., or .., return false. - NS_ConvertUTF16toUTF8 cname(aPath); - char* buffer = cname.BeginWriting(); - const char* token; + } - while ((token = nsCRT::strtok(buffer, "/", &buffer))) { - if (PL_strcmp(token, "") == 0 || - PL_strcmp(token, ".") == 0 || - PL_strcmp(token, "..") == 0 ) { + NS_NAMED_LITERAL_STRING(kCurrentDir, "."); + NS_NAMED_LITERAL_STRING(kParentDir, ".."); + + // Split path and check each path component. + nsCharSeparatedTokenizerTemplate + tokenizer(aPath, FILESYSTEM_DOM_PATH_SEPARATOR_CHAR); + + while (tokenizer.hasMoreTokens()) { + nsDependentSubstring pathComponent = tokenizer.nextToken(); + // The path containing empty components, such as "foo//bar", is invalid. + // We don't allow paths, such as "../foo", "foo/./bar" and "foo/../bar", + // to walk up the directory. + if (pathComponent.IsEmpty() || + pathComponent.Equals(kCurrentDir) || + pathComponent.Equals(kParentDir)) { return false; } + + if (aParts) { + aParts->AppendElement(pathComponent); + } } return true; } void -DeviceStorageFile::NormalizeFilePath() { - FileSystemUtils::LocalPathToNormalizedPath(mPath, mPath); -} - -void -DeviceStorageFile::AppendRelativePath(const nsAString& aPath) { +DeviceStorageFile::AppendRelativePath(const nsAString& aPath) +{ if (!mFile) { return; } - if (!IsSafePath(aPath)) { + + nsTArray parts; + + if (!ValidateAndSplitPath(aPath, &parts)) { // All of the APIs (in the child) do checks to verify that the path is // valid and return PERMISSION_DENIED if a non-safe path is entered. // This check is done in the parent and prevents a compromised @@ -800,9 +838,13 @@ DeviceStorageFile::AppendRelativePath(const nsAString& aPath) { NS_WARNING(NS_LossyConvertUTF16toASCII(aPath).get()); return; } - nsString localPath; - FileSystemUtils::NormalizedPathToLocalPath(aPath, localPath); - mFile->AppendRelativePath(localPath); + + for (uint32_t i = 0; i < parts.Length(); ++i) { + nsresult rv = mFile->AppendRelativePath(parts[i]); + if (NS_WARN_IF(NS_FAILED(rv))) { + return; + } + } } nsresult diff --git a/dom/filesystem/CreateFileTask.cpp b/dom/filesystem/CreateFileTask.cpp index 3df2ac818bbc..42766305721c 100644 --- a/dom/filesystem/CreateFileTask.cpp +++ b/dom/filesystem/CreateFileTask.cpp @@ -168,8 +168,8 @@ CreateFileTask::GetRequestParams(const nsString& aSerializedDOMPath, param.replace() = mReplace; if (mBlobData) { - BlobChild* actor - = ContentChild::GetSingleton()->GetOrCreateActorForBlob(mBlobData); + BlobChild* actor = + ContentChild::GetSingleton()->GetOrCreateActorForBlob(mBlobData); if (actor) { param.data() = actor; } @@ -205,7 +205,7 @@ CreateFileTask::SetSuccessRequestResult(const FileSystemResponseValue& aValue, nsresult CreateFileTask::Work() { - class AutoClose + class MOZ_RAII AutoClose final { public: explicit AutoClose(nsIOutputStream* aStream) @@ -218,6 +218,7 @@ CreateFileTask::Work() { mStream->Close(); } + private: nsCOMPtr mStream; }; diff --git a/dom/filesystem/DeviceStorageFileSystem.cpp b/dom/filesystem/DeviceStorageFileSystem.cpp index 8f25b3eaa5c0..06437115e7ad 100644 --- a/dom/filesystem/DeviceStorageFileSystem.cpp +++ b/dom/filesystem/DeviceStorageFileSystem.cpp @@ -21,9 +21,8 @@ namespace mozilla { namespace dom { -DeviceStorageFileSystem::DeviceStorageFileSystem( - const nsAString& aStorageType, - const nsAString& aStorageName) +DeviceStorageFileSystem::DeviceStorageFileSystem(const nsAString& aStorageType, + const nsAString& aStorageName) : mWindowId(0) { MOZ_ASSERT(NS_IsMainThread(), "Only call on main thread!"); @@ -51,9 +50,6 @@ DeviceStorageFileSystem::DeviceStorageFileSystem( return; } - FileSystemUtils::LocalPathToNormalizedPath(mLocalRootPath, - mNormalizedLocalRootPath); - // DeviceStorageTypeChecker is a singleton object and must be initialized on // the main thread. We initialize it here so that we can use it on the worker // thread. @@ -105,12 +101,15 @@ DeviceStorageFileSystem::IsSafeFile(nsIFile* aFile) const "Should be on parent process!"); MOZ_ASSERT(aFile); - // Check if this file belongs to this storage. - nsAutoString path; - if (NS_FAILED(aFile->GetPath(path))) { + nsCOMPtr rootPath; + nsresult rv = NS_NewLocalFile(GetLocalRootPath(), false, + getter_AddRefs(rootPath)); + if (NS_WARN_IF(NS_FAILED(rv))) { return false; } - if (!LocalPathToRealPath(path, path)) { + + // Check if this file belongs to this storage. + if (NS_WARN_IF(!FileSystemUtils::IsDescendantPath(rootPath, aFile))) { return false; } diff --git a/dom/filesystem/Directory.cpp b/dom/filesystem/Directory.cpp index aebcba1a0d2d..fd46ad15a3ef 100644 --- a/dom/filesystem/Directory.cpp +++ b/dom/filesystem/Directory.cpp @@ -37,6 +37,12 @@ namespace dom { namespace { +bool +TokenizerIgnoreNothing(char16_t /* aChar */) +{ + return false; +} + bool IsValidRelativeDOMPath(const nsString& aPath, nsTArray& aParts) { @@ -55,7 +61,9 @@ IsValidRelativeDOMPath(const nsString& aPath, nsTArray& aParts) NS_NAMED_LITERAL_STRING(kParentDir, ".."); // Split path and check each path component. - nsCharSeparatedTokenizer tokenizer(aPath, FILESYSTEM_DOM_PATH_SEPARATOR_CHAR); + nsCharSeparatedTokenizerTemplate + tokenizer(aPath, FILESYSTEM_DOM_PATH_SEPARATOR_CHAR); + while (tokenizer.hasMoreTokens()) { nsDependentSubstring pathComponent = tokenizer.nextToken(); // The path containing empty components, such as "foo//bar", is invalid. diff --git a/dom/filesystem/FileSystemBase.cpp b/dom/filesystem/FileSystemBase.cpp index b1147759acea..3d6e4529ae39 100644 --- a/dom/filesystem/FileSystemBase.cpp +++ b/dom/filesystem/FileSystemBase.cpp @@ -99,20 +99,5 @@ FileSystemBase::IsSafeDirectory(Directory* aDir) const return false; } -bool -FileSystemBase::LocalPathToRealPath(const nsAString& aLocalPath, - nsAString& aRealPath) const -{ - nsAutoString path; - FileSystemUtils::LocalPathToNormalizedPath(aLocalPath, path); - if (!FileSystemUtils::IsDescendantPath(mNormalizedLocalRootPath, path)) { - aRealPath.Truncate(); - return false; - } - - aRealPath = Substring(path, mNormalizedLocalRootPath.Length()); - return true; -} - } // namespace dom } // namespace mozilla diff --git a/dom/filesystem/FileSystemBase.h b/dom/filesystem/FileSystemBase.h index a01c84cdd11c..eb2c782baa92 100644 --- a/dom/filesystem/FileSystemBase.h +++ b/dom/filesystem/FileSystemBase.h @@ -87,18 +87,12 @@ public: protected: virtual ~FileSystemBase(); - bool - LocalPathToRealPath(const nsAString& aLocalPath, nsAString& aRealPath) const; - // The local path of the root (i.e. the OS path, with OS path separators, of // the OS directory that acts as the root of this OSFileSystem). // Only available in the parent process. // In the child process, we don't use it and its value should be empty. nsString mLocalRootPath; - // The same, but with path separators normalized to "/". - nsString mNormalizedLocalRootPath; - bool mShutdown; // The permission name required to access the file system. diff --git a/dom/filesystem/FileSystemPermissionRequest.cpp b/dom/filesystem/FileSystemPermissionRequest.cpp index 488b0c14de2e..b2c8a9d82769 100644 --- a/dom/filesystem/FileSystemPermissionRequest.cpp +++ b/dom/filesystem/FileSystemPermissionRequest.cpp @@ -15,7 +15,8 @@ namespace mozilla { namespace dom { -NS_IMPL_ISUPPORTS(FileSystemPermissionRequest, nsIRunnable, nsIContentPermissionRequest) +NS_IMPL_ISUPPORTS(FileSystemPermissionRequest, nsIRunnable, + nsIContentPermissionRequest) // static void @@ -28,8 +29,7 @@ FileSystemPermissionRequest::RequestForTask(FileSystemTaskBase* aTask) NS_DispatchToCurrentThread(request); } -FileSystemPermissionRequest::FileSystemPermissionRequest( - FileSystemTaskBase* aTask) +FileSystemPermissionRequest::FileSystemPermissionRequest(FileSystemTaskBase* aTask) : mTask(aTask) { MOZ_ASSERT(mTask, "aTask should not be null!"); diff --git a/dom/filesystem/FileSystemTaskBase.cpp b/dom/filesystem/FileSystemTaskBase.cpp index df11f3512c01..46cae6d86c0e 100644 --- a/dom/filesystem/FileSystemTaskBase.cpp +++ b/dom/filesystem/FileSystemTaskBase.cpp @@ -23,7 +23,7 @@ namespace dom { namespace { -class FileSystemReleaseRunnable : public nsRunnable +class FileSystemReleaseRunnable final : public nsRunnable { public: explicit FileSystemReleaseRunnable(RefPtr& aDoomed) diff --git a/dom/filesystem/FileSystemUtils.cpp b/dom/filesystem/FileSystemUtils.cpp index c5a8817f7cd5..20d2198b2926 100644 --- a/dom/filesystem/FileSystemUtils.cpp +++ b/dom/filesystem/FileSystemUtils.cpp @@ -6,67 +6,10 @@ #include "mozilla/dom/FileSystemUtils.h" -#include "nsXULAppAPI.h" - namespace mozilla { namespace dom { -// static -void -FileSystemUtils::LocalPathToNormalizedPath(const nsAString& aLocal, - nsAString& aNorm) -{ - nsString result; - result = aLocal; -#if defined(XP_WIN) - char16_t* cur = result.BeginWriting(); - char16_t* end = result.EndWriting(); - for (; cur < end; ++cur) { - if (char16_t('\\') == *cur) - *cur = char16_t('/'); - } -#endif - aNorm = result; -} - -// static -void -FileSystemUtils::NormalizedPathToLocalPath(const nsAString& aNorm, - nsAString& aLocal) -{ - nsString result; - result = aNorm; -#if defined(XP_WIN) - char16_t* cur = result.BeginWriting(); - char16_t* end = result.EndWriting(); - for (; cur < end; ++cur) { - if (char16_t('/') == *cur) - *cur = char16_t('\\'); - } -#endif - aLocal = result; -} - -// static -bool -FileSystemUtils::IsDescendantPath(const nsAString& aPath, - const nsAString& aDescendantPath) -{ - // The descendant path should begin with its ancestor path. - nsAutoString prefix; - prefix = aPath + NS_LITERAL_STRING(FILESYSTEM_DOM_PATH_SEPARATOR_LITERAL); - - // Check the sub-directory path to see if it has the parent path as prefix. - if (aDescendantPath.Length() < prefix.Length() || - !StringBeginsWith(aDescendantPath, prefix)) { - return false; - } - - return true; -} - -// static -bool +/* static */ bool FileSystemUtils::IsDescendantPath(nsIFile* aFile, nsIFile* aDescendantFile) { diff --git a/dom/filesystem/FileSystemUtils.h b/dom/filesystem/FileSystemUtils.h index a2572bbf1775..6c1fbc456def 100644 --- a/dom/filesystem/FileSystemUtils.h +++ b/dom/filesystem/FileSystemUtils.h @@ -7,7 +7,7 @@ #ifndef mozilla_dom_FileSystemUtils_h #define mozilla_dom_FileSystemUtils_h -#include "nsString.h" +class nsIFile; namespace mozilla { namespace dom { @@ -22,31 +22,11 @@ namespace dom { class FileSystemUtils { public: - /* - * Convert the path separator to "/". - */ - static void - LocalPathToNormalizedPath(const nsAString& aLocal, nsAString& aNorm); - - /* - * Convert the normalized path separator "/" to the system dependent path - * separator, which is "/" on Mac and Linux, and "\" on Windows. - */ - static void - NormalizedPathToLocalPath(const nsAString& aNorm, nsAString& aLocal); - /* * Return true if aDescendantPath is a descendant of aPath. */ static bool IsDescendantPath(nsIFile* aPath, nsIFile* aDescendantPath); - - /* - * Return true if aDescendantPath is a descendant of aPath. Both aPath and - * aDescendantPath are absolute DOM path. - */ - static bool - IsDescendantPath(const nsAString& aPath, const nsAString& aDescendantPath); }; } // namespace dom diff --git a/dom/filesystem/GetDirectoryListingTask.cpp b/dom/filesystem/GetDirectoryListingTask.cpp index 04406aedbf3f..dd7d566c2af3 100644 --- a/dom/filesystem/GetDirectoryListingTask.cpp +++ b/dom/filesystem/GetDirectoryListingTask.cpp @@ -128,15 +128,6 @@ GetDirectoryListingTask::GetRequestParams(const nsString& aSerializedDOMPath, mFilters); } -void -GetDirectoryListingTask::CreateNormalizedRelativePath(const nsAString& aPath, - nsAString& aRelativePath) const -{ - uint32_t rootPathLen = mFileSystem->GetLocalRootPath().Length(); - FileSystemUtils::LocalPathToNormalizedPath( - Substring(aPath, rootPathLen, aPath.Length() - rootPathLen), aRelativePath); -} - FileSystemResponseValue GetDirectoryListingTask::GetSuccessRequestResult(ErrorResult& aRv) const { @@ -366,10 +357,11 @@ GetDirectoryListingTask::HandlerCallback() MOZ_ASSERT(FileSystemUtils::IsDescendantPath(rootPath, directoryPath)); #endif - RefPtr directory = Directory::Create(mFileSystem->GetParentObject(), - directoryPath, - Directory::eNotDOMRootDirectory, - mFileSystem); + RefPtr directory = + Directory::Create(mFileSystem->GetParentObject(), + directoryPath, + Directory::eNotDOMRootDirectory, + mFileSystem); MOZ_ASSERT(directory); // Propogate mFilter onto sub-Directory object: diff --git a/dom/filesystem/GetDirectoryListingTask.h b/dom/filesystem/GetDirectoryListingTask.h index ddf774e96350..f0a3ae7fd55f 100644 --- a/dom/filesystem/GetDirectoryListingTask.h +++ b/dom/filesystem/GetDirectoryListingTask.h @@ -70,10 +70,6 @@ private: virtual void HandlerCallback() override; - void - CreateNormalizedRelativePath(const nsAString& aPath, - nsAString& aRelativePath) const; - RefPtr mPromise; nsCOMPtr mTargetPath; nsString mFilters; diff --git a/dom/filesystem/OSFileSystem.cpp b/dom/filesystem/OSFileSystem.cpp index 1a7f947280f3..79743e731a1e 100644 --- a/dom/filesystem/OSFileSystem.cpp +++ b/dom/filesystem/OSFileSystem.cpp @@ -20,8 +20,6 @@ namespace dom { OSFileSystem::OSFileSystem(const nsAString& aRootDir) { mLocalRootPath = aRootDir; - FileSystemUtils::LocalPathToNormalizedPath(mLocalRootPath, - mNormalizedLocalRootPath); // Non-mobile devices don't have the concept of separate permissions to // access different parts of devices storage like Pictures, or Videos, etc.