[Backport] CVE-2019-13675

Fixing extension corruption when navigating to extension resource with slash at end

Because of how Content Verifier currently normalizes relative paths of
an extension resource, it (incorrectly) drops any separators at the end
of the relative path. This makes Content Verifier incorrectly think
that a resource exists (if the separators came after a valid extension
resource path) and this results in content verification failure.

Fix this by ensuring content verifier path normalization does not drop
trailing separator, if present.

Bug: 929578

bar.html is present must not corrupt or disable the extension.

Test: Navigating to chrome-extension://<extensionId>/bar.html/ when
Change-Id: I3972643d9f9566e011070e4b01f0b1a50e3fa659
Commit-Queue: Utkarsh Patankar <utkpat@microsoft.com>
Auto-Submit: Utkarsh Patankar <utkpat@microsoft.com>
Reviewed-by: Istiaque Ahmed <lazyboy@chromium.org>
Cr-Commit-Position: refs/heads/master@{#667431}
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

3 files changed, 36 insertions, 2 deletions

diff --git a/chromium/extensions/browser/content_verifier.cc b/chromium/extensions/browser/content_verifier.cc
index 6d87b26ef42..f2d4f660cba 100644
--- a/chromium/extensions/browser/content_verifier.cc
+++ b/chromium/extensions/browser/content_verifier.cc
@@ -59,8 +59,12 @@ base::FilePath NormalizeRelativePath(const base::FilePath& path) {
 
   // Note that elsewhere we always normalize path separators to '/' so this
   // should work for all platforms.
-  return base::FilePath(
-      base::JoinString(parts, base::FilePath::StringType(1, '/')));
+  base::FilePath::StringType normalized_relative_path =
+      base::JoinString(parts, base::FilePath::StringType(1, '/'));
+  // Preserve trailing separator, if present.
+  if (path.EndsWithSeparator())
+    normalized_relative_path.append(1, '/');
+  return base::FilePath(normalized_relative_path);
 }
 
 bool HasScriptFileExt(const base::FilePath& requested_path) {
@@ -751,4 +755,9 @@ void ContentVerifier::ResetIODataForTesting(const Extension* extension) {
   io_data_->AddData(extension->id(), CreateIOData(extension, delegate_.get()));
 }
 
+base::FilePath ContentVerifier::NormalizeRelativePathForTesting(
+    const base::FilePath& path) {
+  return NormalizeRelativePath(path);
+}
+
 }  // namespace extensions
diff --git a/chromium/extensions/browser/content_verifier.h b/chromium/extensions/browser/content_verifier.h
index 24410bdd2e2..8780385de04 100644
--- a/chromium/extensions/browser/content_verifier.h
+++ b/chromium/extensions/browser/content_verifier.h
@@ -102,6 +102,10 @@ class ContentVerifier : public base::RefCountedThreadSafe<ContentVerifier>,
   // call |OnExtensionLoaded|.
   void ResetIODataForTesting(const Extension* extension);
 
+  // Test helper to normalize relative path of file.
+  static base::FilePath NormalizeRelativePathForTesting(
+      const base::FilePath& path);
+
  private:
   friend class ContentVerifierTest;
   friend class base::RefCountedThreadSafe<ContentVerifier>;
diff --git a/chromium/extensions/browser/content_verifier_unittest.cc b/chromium/extensions/browser/content_verifier_unittest.cc
index b1c28495dea..0ccfca5779e 100644
--- a/chromium/extensions/browser/content_verifier_unittest.cc
+++ b/chromium/extensions/browser/content_verifier_unittest.cc
@@ -9,6 +9,7 @@
 #include "base/values.h"
 #include "content/public/test/test_browser_thread_bundle.h"
 #include "content/public/test/test_utils.h"
+#include "extensions/browser/content_verifier.h"
 #include "extensions/browser/content_verifier/test_utils.h"
 #include "extensions/browser/extension_registry.h"
 #include "extensions/browser/extensions_test.h"
@@ -215,4 +216,24 @@ INSTANTIATE_TEST_CASE_P(
                     BackgroundManifestType::kBackgroundScript,
                     BackgroundManifestType::kBackgroundPage));
 
+TEST(ContentVerifierTest, NormalizeRelativePath) {
+// This macro helps avoid wrapped lines in the test structs.
+#define FPL(x) FILE_PATH_LITERAL(x)
+  struct TestData {
+    base::FilePath::StringPieceType input;
+    base::FilePath::StringPieceType expected;
+  } test_cases[] = {{FPL("foo/bar"), FPL("foo/bar")},
+                    {FPL("foo//bar"), FPL("foo/bar")},
+                    {FPL("foo/bar/"), FPL("foo/bar/")},
+                    {FPL("foo/bar//"), FPL("foo/bar/")},
+                    {FPL("foo/options.html/"), FPL("foo/options.html/")}};
+#undef FPL
+  for (const auto& test_case : test_cases) {
+    base::FilePath input(test_case.input);
+    base::FilePath expected(test_case.expected);
+    EXPECT_EQ(expected,
+              ContentVerifier::NormalizeRelativePathForTesting(input));
+  }
+}
+
 }  // namespace extensions