Small extra validations on permission checks: reject "0" as a valid permission

content/browser/child_process_security_policy_unittest.cc

 // Copyright (c) 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include <set>
 #include <string>
 
 #include "base/basictypes.h"
 #include "base/file_path.h"
 #include "base/platform_file.h"
 #include "content/browser/child_process_security_policy_impl.h"
 #include "content/public/common/url_constants.h"
 #include "content/test/test_content_browser_client.h"
 #include "googleurl/src/gurl.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
 namespace content {
 namespace {
 
 const int kRendererID = 42;
 const int kWorkerRendererID = kRendererID + 1;
 
+#if defined(FILE_PATH_USES_DRIVE_LETTERS)
+#define TEST_PATH(x) FILE_PATH_LITERAL("c:") FILE_PATH_LITERAL(x)
+#else
+#define TEST_PATH(x) FILE_PATH_LITERAL(x)
+#endif
+
 class ChildProcessSecurityPolicyTestBrowserClient
     : public TestContentBrowserClient {
  public:
   ChildProcessSecurityPolicyTestBrowserClient() {}
 
   virtual bool IsHandledURL(const GURL& url) {
     return schemes_.find(url.scheme()) != schemes_.end();
   }
 
   void ClearSchemes() {
@@ skipping 254 matching lines @@
 
   p->Remove(kRendererID);
 }
 
 TEST_F(ChildProcessSecurityPolicyTest, CanReadFiles) {
   ChildProcessSecurityPolicyImpl* p =
       ChildProcessSecurityPolicyImpl::GetInstance();
 
   p->Add(kRendererID);
 
-  EXPECT_FALSE(p->CanReadFile(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/passwd"))));
-  p->GrantReadFile(kRendererID, FilePath(FILE_PATH_LITERAL("/etc/passwd")));
-  EXPECT_TRUE(p->CanReadFile(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/passwd"))));
-  EXPECT_FALSE(p->CanReadFile(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/shadow"))));
+  EXPECT_FALSE(p->CanReadFile(kRendererID, FilePath(TEST_PATH("/etc/passwd"))));
+  p->GrantReadFile(kRendererID, FilePath(TEST_PATH("/etc/passwd")));
+  EXPECT_TRUE(p->CanReadFile(kRendererID, FilePath(TEST_PATH("/etc/passwd"))));
+  EXPECT_FALSE(p->CanReadFile(kRendererID, FilePath(TEST_PATH("/etc/shadow"))));
 
   p->Remove(kRendererID);
   p->Add(kRendererID);
 
-  EXPECT_FALSE(p->CanReadFile(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/passwd"))));
-  EXPECT_FALSE(p->CanReadFile(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/shadow"))));
+  EXPECT_FALSE(p->CanReadFile(kRendererID, FilePath(TEST_PATH("/etc/passwd"))));
+  EXPECT_FALSE(p->CanReadFile(kRendererID, FilePath(TEST_PATH("/etc/shadow"))));
 
   p->Remove(kRendererID);
 }
 
 TEST_F(ChildProcessSecurityPolicyTest, CanReadDirectories) {
   ChildProcessSecurityPolicyImpl* p =
       ChildProcessSecurityPolicyImpl::GetInstance();
 
   p->Add(kRendererID);
 
-  EXPECT_FALSE(p->CanReadDirectory(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/"))));
-  p->GrantReadDirectory(kRendererID, FilePath(FILE_PATH_LITERAL("/etc/")));
-  EXPECT_TRUE(p->CanReadDirectory(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/"))));
-  EXPECT_TRUE(p->CanReadFile(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/passwd"))));
+  EXPECT_FALSE(p->CanReadDirectory(kRendererID, FilePath(TEST_PATH("/etc/"))));
+  p->GrantReadDirectory(kRendererID, FilePath(TEST_PATH("/etc/")));
+  EXPECT_TRUE(p->CanReadDirectory(kRendererID, FilePath(TEST_PATH("/etc/"))));
+  EXPECT_TRUE(p->CanReadFile(kRendererID, FilePath(TEST_PATH("/etc/passwd"))));
 
   p->Remove(kRendererID);
   p->Add(kRendererID);
 
-  EXPECT_FALSE(p->CanReadDirectory(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/"))));
-  EXPECT_FALSE(p->CanReadFile(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/passwd"))));
+  EXPECT_FALSE(p->CanReadDirectory(kRendererID, FilePath(TEST_PATH("/etc/"))));
+  EXPECT_FALSE(p->CanReadFile(kRendererID, FilePath(TEST_PATH("/etc/passwd"))));
 
   // Just granting read permission as a file doesn't imply reading as a
   // directory.
-  p->GrantReadFile(kRendererID, FilePath(FILE_PATH_LITERAL("/etc/")));
-  EXPECT_TRUE(p->CanReadFile(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/passwd"))));
-  EXPECT_FALSE(p->CanReadDirectory(kRendererID,
-      FilePath(FILE_PATH_LITERAL("/etc/"))));
+  p->GrantReadFile(kRendererID, FilePath(TEST_PATH("/etc/")));
+  EXPECT_TRUE(p->CanReadFile(kRendererID, FilePath(TEST_PATH("/etc/passwd"))));
+  EXPECT_FALSE(p->CanReadDirectory(kRendererID, FilePath(TEST_PATH("/etc/"))));
 
   p->Remove(kRendererID);
 }
 
 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 granted_file = FilePath(TEST_PATH("/home/joe"));
+  FilePath sibling_file = FilePath(TEST_PATH("/home/bob"));
+  FilePath child_file = FilePath(TEST_PATH("/home/joe/file"));
+  FilePath parent_file = FilePath(TEST_PATH("/home"));
+  FilePath parent_slash_file = FilePath(TEST_PATH("/home/"));
+  FilePath child_traversal1 = FilePath(TEST_PATH("/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"));
+      TEST_PATH("/home/joe/file/../otherfile"));
+  FilePath evil_traversal1 = FilePath(TEST_PATH("/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"));
+      TEST_PATH("/home/joe/./.././../etc/passwd"));
+  FilePath self_traversal = FilePath(TEST_PATH("/home/joe/../joe/file"));
+  FilePath relative_file = FilePath(TEST_PATH("home/joe"));

[Charlie Reis, 2013/01/04 20:59:34]

Isn't this going to result in c:home/joe?  I suppose that's worth testing, but
so is "home/joe".

 
   ChildProcessSecurityPolicyImpl* p =
       ChildProcessSecurityPolicyImpl::GetInstance();
 
   // Grant permissions for a file.
   p->Add(kRendererID);
   EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file,
                                         base::PLATFORM_FILE_OPEN));
 
   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, granted_file,
                                        base::PLATFORM_FILE_OPEN |
                                        base::PLATFORM_FILE_OPEN_TRUNCATED |
                                        base::PLATFORM_FILE_READ |
                                        base::PLATFORM_FILE_WRITE));
   EXPECT_TRUE(p->HasPermissionsForFile(kRendererID, granted_file,
                                        base::PLATFORM_FILE_OPEN |
                                        base::PLATFORM_FILE_READ));
   EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file,
                                         base::PLATFORM_FILE_CREATE));
+  EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, granted_file, 0));
   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 |
@@ skipping 80 matching lines @@
   EXPECT_TRUE(p->HasPermissionsForFile(kWorkerRendererID, granted_file,
                                        base::PLATFORM_FILE_OPEN |
                                        base::PLATFORM_FILE_READ));
   EXPECT_FALSE(p->HasPermissionsForFile(kWorkerRendererID, granted_file,
                                         base::PLATFORM_FILE_WRITE));
   p->Remove(kRendererID);
   EXPECT_FALSE(p->HasPermissionsForFile(kWorkerRendererID, granted_file,
                                         base::PLATFORM_FILE_OPEN |
                                         base::PLATFORM_FILE_READ));
   p->Remove(kWorkerRendererID);
+
+  p->Add(kRendererID);
+  p->GrantPermissionsForFile(kRendererID, relative_file,
+                             base::PLATFORM_FILE_OPEN);
+  EXPECT_FALSE(p->HasPermissionsForFile(kRendererID, relative_file,
+                                        base::PLATFORM_FILE_OPEN));
+  p->Remove(kRendererID);
 }
 
 TEST_F(ChildProcessSecurityPolicyTest, CanServiceWebUIBindings) {
   ChildProcessSecurityPolicyImpl* p =
       ChildProcessSecurityPolicyImpl::GetInstance();
 
   GURL url("chrome://thumb/http://www.google.com/");
 
   p->Add(kRendererID);
 
   EXPECT_FALSE(p->HasWebUIBindings(kRendererID));
   EXPECT_FALSE(p->CanRequestURL(kRendererID, url));
   p->GrantWebUIBindings(kRendererID);
   EXPECT_TRUE(p->HasWebUIBindings(kRendererID));
   EXPECT_TRUE(p->CanRequestURL(kRendererID, url));
 
   p->Remove(kRendererID);
 }
 
 TEST_F(ChildProcessSecurityPolicyTest, RemoveRace) {
   ChildProcessSecurityPolicyImpl* p =
       ChildProcessSecurityPolicyImpl::GetInstance();
 
   GURL url("file:///etc/passwd");
-  FilePath file(FILE_PATH_LITERAL("/etc/passwd"));
+  FilePath file(TEST_PATH("/etc/passwd"));
 
   p->Add(kRendererID);
 
   p->GrantRequestURL(kRendererID, url);
   p->GrantReadFile(kRendererID, file);
   p->GrantWebUIBindings(kRendererID);
 
   EXPECT_TRUE(p->CanRequestURL(kRendererID, url));
   EXPECT_TRUE(p->CanReadFile(kRendererID, file));
   EXPECT_TRUE(p->HasWebUIBindings(kRendererID));
 
   p->Remove(kRendererID);
 
   // Renderers are added and removed on the UI thread, but the policy can be
   // queried on the IO thread.  The ChildProcessSecurityPolicy needs to be
   // prepared to answer policy questions about renderers who no longer exist.
 
   // In this case, we default to secure behavior.
   EXPECT_FALSE(p->CanRequestURL(kRendererID, url));
   EXPECT_FALSE(p->CanReadFile(kRendererID, file));
   EXPECT_FALSE(p->HasWebUIBindings(kRendererID));
 }
 
 }  // namespace content