diff options
author | tsepez@chromium.org <tsepez@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-20 01:53:59 +0000 |
---|---|---|
committer | tsepez@chromium.org <tsepez@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2012-11-20 01:53:59 +0000 |
commit | 8083841913b8eb8018ae52f67c923f0b3d66c466 (patch) | |
tree | 0f039083e00ffc86c50ff42842b2abf579df3ac7 /content | |
parent | e999db5466b40e5e915961585e4856524753a9e9 (diff) | |
download | chromium_src-8083841913b8eb8018ae52f67c923f0b3d66c466.zip chromium_src-8083841913b8eb8018ae52f67c923f0b3d66c466.tar.gz chromium_src-8083841913b8eb8018ae52f67c923f0b3d66c466.tar.bz2 |
Apply missing kParentDirectory check
BUG=161564
Review URL: https://chromiumcodereview.appspot.com/11414046
git-svn-id: svn://svn.chromium.org/chrome/trunk/src@168692 0039d316-1c4b-4281-b951-d872f2087c98
Diffstat (limited to 'content')
-rw-r--r-- | content/browser/child_process_security_policy_impl.cc | 13 | ||||
-rw-r--r-- | content/browser/child_process_security_policy_unittest.cc | 98 |
2 files changed, 81 insertions, 30 deletions
diff --git a/content/browser/child_process_security_policy_impl.cc b/content/browser/child_process_security_policy_impl.cc index 006418e3..e86ccae 100644 --- a/content/browser/child_process_security_policy_impl.cc +++ b/content/browser/child_process_security_policy_impl.cc @@ -148,9 +148,18 @@ class ChildProcessSecurityPolicyImpl::SecurityState { bool HasPermissionsForFile(const FilePath& file, int permissions) { FilePath current_path = file.StripTrailingSeparators(); FilePath last_path; + int skip = 0; while (current_path != last_path) { - if (file_permissions_.find(current_path) != file_permissions_.end()) - return (file_permissions_[current_path] & permissions) == permissions; + FilePath base_name = current_path.BaseName(); + if (base_name.value() == FilePath::kParentDirectory) { + ++skip; + } else if (skip > 0) { + if (base_name.value() != FilePath::kCurrentDirectory) + --skip; + } else { + if (file_permissions_.find(current_path) != file_permissions_.end()) + return (file_permissions_[current_path] & permissions) == permissions; + } last_path = current_path; current_path = current_path.DirName(); } diff --git a/content/browser/child_process_security_policy_unittest.cc b/content/browser/child_process_security_policy_unittest.cc index 0cd3a3f..90befd6 100644 --- a/content/browser/child_process_security_policy_unittest.cc +++ b/content/browser/child_process_security_policy_unittest.cc @@ -347,100 +347,142 @@ TEST_F(ChildProcessSecurityPolicyTest, CanReadDirectories) { } TEST_F(ChildProcessSecurityPolicyTest, FilePermissions) { + FilePath granted_file = FilePath(FILE_PATH_LITERAL("/home/joe")); + FilePath sibling_file = FilePath(FILE_PATH_LITERAL("/home/bob")); + FilePath child_file = FilePath(FILE_PATH_LITERAL("/home/joe/file")); + FilePath parent_file = FilePath(FILE_PATH_LITERAL("/home")); + FilePath parent_slash_file = FilePath(FILE_PATH_LITERAL("/home/")); + FilePath child_traversal1 = FilePath( + FILE_PATH_LITERAL("/home/joe/././file")); + FilePath child_traversal2 = FilePath( + FILE_PATH_LITERAL("/home/joe/file/../otherfile")); + FilePath evil_traversal1 = FilePath( + FILE_PATH_LITERAL("/home/joe/../../etc/passwd")); + FilePath evil_traversal2 = FilePath( + FILE_PATH_LITERAL("/home/joe/./.././../etc/passwd")); + FilePath self_traversal = FilePath( + FILE_PATH_LITERAL("/home/joe/../joe/file")); + ChildProcessSecurityPolicyImpl* p = ChildProcessSecurityPolicyImpl::GetInstance(); // Grant permissions for a file. p->Add(kRendererID); - FilePath file = FilePath(FILE_PATH_LITERAL("/etc/passwd")); - EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_OPEN)); - p->GrantPermissionsForFile(kRendererID, file, + p->GrantPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_OPEN_TRUNCATED | base::PLATFORM_FILE_READ | base::PLATFORM_FILE_WRITE); - EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_OPEN_TRUNCATED | base::PLATFORM_FILE_READ | base::PLATFORM_FILE_WRITE)); - EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ)); - EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_CREATE)); - EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_CREATE | base::PLATFORM_FILE_OPEN_TRUNCATED | base::PLATFORM_FILE_READ | base::PLATFORM_FILE_WRITE)); + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, sibling_file, + base::PLATFORM_FILE_OPEN | + base::PLATFORM_FILE_READ)); + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, parent_file, + base::PLATFORM_FILE_OPEN | + base::PLATFORM_FILE_READ)); + EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, child_file, + base::PLATFORM_FILE_OPEN | + base::PLATFORM_FILE_READ)); + EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, child_traversal1, + base::PLATFORM_FILE_OPEN | + base::PLATFORM_FILE_READ)); + EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, child_traversal2, + base::PLATFORM_FILE_OPEN | + base::PLATFORM_FILE_READ)); + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, evil_traversal1, + base::PLATFORM_FILE_OPEN | + base::PLATFORM_FILE_READ)); + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, evil_traversal2, + base::PLATFORM_FILE_OPEN | + base::PLATFORM_FILE_READ)); + // CPSP doesn't allow this case for the sake of simplicity. + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, self_traversal, + base::PLATFORM_FILE_OPEN | + base::PLATFORM_FILE_READ)); p->Remove(kRendererID); // Grant permissions for the directory the file is in. p->Add(kRendererID); - EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_OPEN)); - p->GrantPermissionsForFile(kRendererID, FilePath(FILE_PATH_LITERAL("/etc")), + p->GrantPermissionsForFile(kRendererID, parent_file, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ); - EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_OPEN)); - EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_READ | base::PLATFORM_FILE_WRITE)); p->Remove(kRendererID); // Grant permissions for the directory the file is in (with trailing '/'). p->Add(kRendererID); - EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_OPEN)); - p->GrantPermissionsForFile(kRendererID, FilePath(FILE_PATH_LITERAL("/etc/")), + p->GrantPermissionsForFile(kRendererID, parent_slash_file, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ); - EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_OPEN)); - EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_READ | base::PLATFORM_FILE_WRITE)); // Grant permissions for the file (should overwrite the permissions granted // for the directory). - p->GrantPermissionsForFile(kRendererID, file, base::PLATFORM_FILE_TEMPORARY); - EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file, + p->GrantPermissionsForFile(kRendererID, granted_file, + base::PLATFORM_FILE_TEMPORARY); + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_OPEN)); - EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_TEMPORARY)); // Revoke all permissions for the file (it should inherit its permissions // from the directory again). - p->RevokeAllPermissionsForFile(kRendererID, file); - EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file, + p->RevokeAllPermissionsForFile(kRendererID, granted_file); + EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ)); - EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_TEMPORARY)); p->Remove(kRendererID); // Grant file permissions for the file to main thread renderer process, // make sure its worker thread renderer process inherits those. p->Add(kRendererID); - p->GrantPermissionsForFile(kRendererID, file, base::PLATFORM_FILE_OPEN | - base::PLATFORM_FILE_READ); - EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, file, + p->GrantPermissionsForFile(kRendererID, granted_file, + base::PLATFORM_FILE_OPEN | + base::PLATFORM_FILE_READ); + EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ)); - EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, file, + EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file, base::PLATFORM_FILE_WRITE)); p->AddWorker(kWorkerRendererID, kRendererID); - EXPECT_TRUE(p->HasPermissionsForFile(kWorkerRendererID, file, + EXPECT_TRUE(p->HasPermissionsForFile(kWorkerRendererID, granted_file, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ)); - EXPECT_FALSE(p->HasPermissionsForFile(kWorkerRendererID, file, + EXPECT_FALSE(p->HasPermissionsForFile(kWorkerRendererID, granted_file, base::PLATFORM_FILE_WRITE)); p->Remove(kRendererID); - EXPECT_FALSE(p->HasPermissionsForFile(kWorkerRendererID, file, + EXPECT_FALSE(p->HasPermissionsForFile(kWorkerRendererID, granted_file, base::PLATFORM_FILE_OPEN | base::PLATFORM_FILE_READ)); p->Remove(kWorkerRendererID); |