Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(225)

Issues Search
    My Issues | Starred     Open | Closed | All

Unified Diff: base/file_util_unittest.cc

Issue 8318011: Update write checks for external extension file on mac. (Closed) Base URL: svn://svn.chromium.org/chrome/trunk/src
Patch Set: Rev comments. Created 9 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « base/file_util_posix.cc ('k') | chrome/common/extensions/docs/external_extensions.html » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: base/file_util_unittest.cc
diff --git a/base/file_util_unittest.cc b/base/file_util_unittest.cc
index b76938e6d8ddb48fa44074e9c70117d78011dcc0..8ac10762467ac482698bd6da136c63d16179090f 100644
--- a/base/file_util_unittest.cc
+++ b/base/file_util_unittest.cc
@@ -1867,7 +1867,9 @@ class VerifyPathControlledByUserTest : public FileUtilTest {
struct stat stat_buf;
ASSERT_EQ(0, stat(base_dir_.value().c_str(), &stat_buf));
uid_ = stat_buf.st_uid;
- gid_ = stat_buf.st_gid;
+ ok_gids_.insert(stat_buf.st_gid);
+ bad_gids_.insert(stat_buf.st_gid + 1);
+
ASSERT_EQ(uid_, getuid()); // This process should be the owner.
// To ensure that umask settings do not cause the initial state
@@ -1892,7 +1894,9 @@ class VerifyPathControlledByUserTest : public FileUtilTest {
FilePath sub_dir_;
FilePath text_file_;
uid_t uid_;
- gid_t gid_;
+
+ std::set<gid_t> ok_gids_;
+ std::set<gid_t> bad_gids_;
};
TEST_F(VerifyPathControlledByUserTest, BadPaths) {
@@ -1900,23 +1904,25 @@ TEST_F(VerifyPathControlledByUserTest, BadPaths) {
FilePath does_not_exist = base_dir_.AppendASCII("does")
.AppendASCII("not")
.AppendASCII("exist");
-
EXPECT_FALSE(
file_util::VerifyPathControlledByUser(
- base_dir_, does_not_exist, uid_, gid_));
+ base_dir_, does_not_exist, uid_, ok_gids_));
// |base| not a subpath of |path|.
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(sub_dir_, base_dir_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, base_dir_, uid_, ok_gids_));
// An empty base path will fail to be a prefix for any path.
FilePath empty;
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(empty, base_dir_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ empty, base_dir_, uid_, ok_gids_));
// Finding that a bad call fails proves nothing unless a good call succeeds.
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(base_dir_, sub_dir_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, ok_gids_));
}
TEST_F(VerifyPathControlledByUserTest, Symlinks) {
@@ -1928,9 +1934,11 @@ TEST_F(VerifyPathControlledByUserTest, Symlinks) {
<< "Failed to create symlink.";
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(base_dir_, file_link, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, file_link, uid_, ok_gids_));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(file_link, file_link, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ file_link, file_link, uid_, ok_gids_));
// Symlink from one directory to another within the path.
FilePath link_to_sub_dir = base_dir_.AppendASCII("link_to_sub_dir");
@@ -1942,25 +1950,22 @@ TEST_F(VerifyPathControlledByUserTest, Symlinks) {
EXPECT_FALSE(
file_util::VerifyPathControlledByUser(
- base_dir_, file_path_with_link, uid_, gid_));
+ base_dir_, file_path_with_link, uid_, ok_gids_));
EXPECT_FALSE(
file_util::VerifyPathControlledByUser(
- link_to_sub_dir, file_path_with_link, uid_, gid_));
+ link_to_sub_dir, file_path_with_link, uid_, ok_gids_));
// Symlinks in parents of base path are allowed.
EXPECT_TRUE(
file_util::VerifyPathControlledByUser(
- file_path_with_link, file_path_with_link, uid_, gid_));
+ file_path_with_link, file_path_with_link, uid_, ok_gids_));
}
TEST_F(VerifyPathControlledByUserTest, OwnershipChecks) {
// Get a uid that is not the uid of files we create.
uid_t bad_uid = uid_ + 1;
- // Get a gid that is not ours.
- gid_t bad_gid = gid_ + 1;
-
// Make all files and directories non-world-writable.
ASSERT_NO_FATAL_FAILURE(
ChangePosixFilePermissions(base_dir_, 0u, S_IWOTH));
@@ -1971,33 +1976,130 @@ TEST_F(VerifyPathControlledByUserTest, OwnershipChecks) {
// We control these paths.
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(base_dir_, sub_dir_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, ok_gids_));
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(base_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, ok_gids_));
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(sub_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, ok_gids_));
// Another user does not control these paths.
EXPECT_FALSE(
file_util::VerifyPathControlledByUser(
- base_dir_, sub_dir_, bad_uid, gid_));
+ base_dir_, sub_dir_, bad_uid, ok_gids_));
EXPECT_FALSE(
file_util::VerifyPathControlledByUser(
- base_dir_, text_file_, bad_uid, gid_));
+ base_dir_, text_file_, bad_uid, ok_gids_));
EXPECT_FALSE(
file_util::VerifyPathControlledByUser(
- sub_dir_, text_file_, bad_uid, gid_));
+ sub_dir_, text_file_, bad_uid, ok_gids_));
// Another group does not control the paths.
EXPECT_FALSE(
file_util::VerifyPathControlledByUser(
- base_dir_, sub_dir_, uid_, bad_gid));
+ base_dir_, sub_dir_, uid_, bad_gids_));
+ EXPECT_FALSE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, bad_gids_));
+ EXPECT_FALSE(
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, bad_gids_));
+}
+
+TEST_F(VerifyPathControlledByUserTest, GroupWriteTest) {
+ // Make all files and directories writable only by their owner.
+ ASSERT_NO_FATAL_FAILURE(
+ ChangePosixFilePermissions(base_dir_, 0u, S_IWOTH|S_IWGRP));
+ ASSERT_NO_FATAL_FAILURE(
+ ChangePosixFilePermissions(sub_dir_, 0u, S_IWOTH|S_IWGRP));
+ ASSERT_NO_FATAL_FAILURE(
+ ChangePosixFilePermissions(text_file_, 0u, S_IWOTH|S_IWGRP));
+
+ // Any group is okay because the path is not group-writable.
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, ok_gids_));
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, ok_gids_));
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, ok_gids_));
+
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, bad_gids_));
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, bad_gids_));
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, bad_gids_));
+
+ // No group is okay, because we don't check the group
+ // if no group can write.
+ std::set<gid_t> no_gids; // Empty set of gids.
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, no_gids));
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, no_gids));
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, no_gids));
+
+
+ // Make all files and directories writable by their group.
+ ASSERT_NO_FATAL_FAILURE(
+ ChangePosixFilePermissions(base_dir_, S_IWGRP, 0u));
+ ASSERT_NO_FATAL_FAILURE(
+ ChangePosixFilePermissions(sub_dir_, S_IWGRP, 0u));
+ ASSERT_NO_FATAL_FAILURE(
+ ChangePosixFilePermissions(text_file_, S_IWGRP, 0u));
+
+ // Now |ok_gids_| works, but |bad_gids_| fails.
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, ok_gids_));
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, ok_gids_));
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, ok_gids_));
+
+ EXPECT_FALSE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, bad_gids_));
EXPECT_FALSE(
file_util::VerifyPathControlledByUser(
- base_dir_, text_file_, uid_, bad_gid));
+ base_dir_, text_file_, uid_, bad_gids_));
EXPECT_FALSE(
file_util::VerifyPathControlledByUser(
- sub_dir_, text_file_, uid_, bad_gid));
+ sub_dir_, text_file_, uid_, bad_gids_));
+
+ // Because any group in the group set is allowed,
+ // the union of good and bad gids passes.
+
+ std::set<gid_t> multiple_gids;
+ std::set_union(
+ ok_gids_.begin(), ok_gids_.end(),
+ bad_gids_.begin(), bad_gids_.end(),
+ std::inserter(multiple_gids, multiple_gids.begin()));
+
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, multiple_gids));
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, multiple_gids));
+ EXPECT_TRUE(
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, multiple_gids));
+
}
TEST_F(VerifyPathControlledByUserTest, WriteBitChecks) {
@@ -2011,72 +2113,93 @@ TEST_F(VerifyPathControlledByUserTest, WriteBitChecks) {
// Initialy, we control all parts of the path.
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(base_dir_, sub_dir_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, ok_gids_));
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(base_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, ok_gids_));
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(sub_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, ok_gids_));
// Make base_dir_ world-writable.
ASSERT_NO_FATAL_FAILURE(
ChangePosixFilePermissions(base_dir_, S_IWOTH, 0u));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(base_dir_, sub_dir_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, ok_gids_));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(base_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, ok_gids_));
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(sub_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, ok_gids_));
// Make sub_dir_ world writable.
ASSERT_NO_FATAL_FAILURE(
ChangePosixFilePermissions(sub_dir_, S_IWOTH, 0u));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(base_dir_, sub_dir_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, ok_gids_));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(base_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, ok_gids_));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(sub_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, ok_gids_));
// Make text_file_ world writable.
ASSERT_NO_FATAL_FAILURE(
ChangePosixFilePermissions(text_file_, S_IWOTH, 0u));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(base_dir_, sub_dir_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, ok_gids_));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(base_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, ok_gids_));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(sub_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, ok_gids_));
// Make sub_dir_ non-world writable.
ASSERT_NO_FATAL_FAILURE(
ChangePosixFilePermissions(sub_dir_, 0u, S_IWOTH));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(base_dir_, sub_dir_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, ok_gids_));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(base_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, ok_gids_));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(sub_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, ok_gids_));
// Make base_dir_ non-world-writable.
ASSERT_NO_FATAL_FAILURE(
ChangePosixFilePermissions(base_dir_, 0u, S_IWOTH));
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(base_dir_, sub_dir_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, ok_gids_));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(base_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, ok_gids_));
EXPECT_FALSE(
- file_util::VerifyPathControlledByUser(sub_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, ok_gids_));
// Back to the initial state: Nothing is writable, so every path
// should pass.
ASSERT_NO_FATAL_FAILURE(
ChangePosixFilePermissions(text_file_, 0u, S_IWOTH));
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(base_dir_, sub_dir_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, sub_dir_, uid_, ok_gids_));
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(base_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ base_dir_, text_file_, uid_, ok_gids_));
EXPECT_TRUE(
- file_util::VerifyPathControlledByUser(sub_dir_, text_file_, uid_, gid_));
+ file_util::VerifyPathControlledByUser(
+ sub_dir_, text_file_, uid_, ok_gids_));
}
#endif // defined(OS_POSIX)
« no previous file with comments | « base/file_util_posix.cc ('k') | chrome/common/extensions/docs/external_extensions.html » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698