summaryrefslogtreecommitdiffstats
path: root/content
diff options
context:
space:
mode:
authortsepez@chromium.org <tsepez@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-11-20 01:53:59 +0000
committertsepez@chromium.org <tsepez@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>2012-11-20 01:53:59 +0000
commit8083841913b8eb8018ae52f67c923f0b3d66c466 (patch)
tree0f039083e00ffc86c50ff42842b2abf579df3ac7 /content
parente999db5466b40e5e915961585e4856524753a9e9 (diff)
downloadchromium_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.cc13
-rw-r--r--content/browser/child_process_security_policy_unittest.cc98
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);