Commit c2800fbb authored by Istiaque Ahmed's avatar Istiaque Ahmed Committed by Commit Bot

ContentVerifier: Fix forced verification of JS/HTML resources.

ContentVerifier::ShouldVerifyAnyPaths forces verification of
JavaScript and html files by checking the extension of the
resource (.js or .html/.htm) as they are considered sensitive
resources.

However, it didn't used to take case variants of extension into
account, this CL fixes that by turning comparison of extension
of files case-insensitive on all platforms.

Added unit test for the behavior change.

Bug: 1051396
Test: See bug description
Change-Id: I0fed253ad5241db5b89c8abf6d0ab445dd403b0f
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2071486
Commit-Queue: Istiaque Ahmed <lazyboy@chromium.org>
Reviewed-by: default avatarOleg Davydov <burunduk@chromium.org>
Cr-Commit-Position: refs/heads/master@{#744744}
parent 92887480
...@@ -70,13 +70,13 @@ base::FilePath NormalizeRelativePath(const base::FilePath& path) { ...@@ -70,13 +70,13 @@ base::FilePath NormalizeRelativePath(const base::FilePath& path) {
} }
bool HasScriptFileExt(const base::FilePath& requested_path) { bool HasScriptFileExt(const base::FilePath& requested_path) {
return requested_path.Extension() == FILE_PATH_LITERAL(".js"); return requested_path.MatchesExtension(FILE_PATH_LITERAL(".js"));
} }
bool HasPageFileExt(const base::FilePath& requested_path) { bool HasPageFileExt(const base::FilePath& requested_path) {
base::FilePath::StringType file_extension = requested_path.Extension(); base::FilePath::StringType file_extension = requested_path.Extension();
return file_extension == FILE_PATH_LITERAL(".html") || return requested_path.MatchesExtension(FILE_PATH_LITERAL(".html")) ||
file_extension == FILE_PATH_LITERAL(".htm"); requested_path.MatchesExtension(FILE_PATH_LITERAL(".htm"));
} }
std::unique_ptr<ContentVerifierIOData::ExtensionData> CreateIOData( std::unique_ptr<ContentVerifierIOData::ExtensionData> CreateIOData(
...@@ -771,4 +771,12 @@ base::FilePath ContentVerifier::NormalizeRelativePathForTesting( ...@@ -771,4 +771,12 @@ base::FilePath ContentVerifier::NormalizeRelativePathForTesting(
return NormalizeRelativePath(path); return NormalizeRelativePath(path);
} }
bool ContentVerifier::ShouldVerifyAnyPathsForTesting(
const std::string& extension_id,
const base::FilePath& extension_root,
const std::set<base::FilePath>& relative_unix_paths) {
return ShouldVerifyAnyPaths(extension_id, extension_root,
relative_unix_paths);
}
} // namespace extensions } // namespace extensions
...@@ -134,8 +134,12 @@ class ContentVerifier : public base::RefCountedThreadSafe<ContentVerifier>, ...@@ -134,8 +134,12 @@ class ContentVerifier : public base::RefCountedThreadSafe<ContentVerifier>,
static base::FilePath NormalizeRelativePathForTesting( static base::FilePath NormalizeRelativePathForTesting(
const base::FilePath& path); const base::FilePath& path);
bool ShouldVerifyAnyPathsForTesting(
const std::string& extension_id,
const base::FilePath& extension_root,
const std::set<base::FilePath>& relative_unix_paths);
private: private:
friend class ContentVerifierTest;
friend class base::RefCountedThreadSafe<ContentVerifier>; friend class base::RefCountedThreadSafe<ContentVerifier>;
friend class HashHelper; friend class HashHelper;
~ContentVerifier() override; ~ContentVerifier() override;
......
...@@ -70,15 +70,15 @@ void TestContentVerifierDelegate::SetBrowserImagePaths( ...@@ -70,15 +70,15 @@ void TestContentVerifierDelegate::SetBrowserImagePaths(
} // namespace } // namespace
class ContentVerifierTest class ContentVerifierTest : public ExtensionsTest {
: public ExtensionsTest,
public testing::WithParamInterface<BackgroundManifestType> {
public: public:
ContentVerifierTest() {} ContentVerifierTest() = default;
ContentVerifierTest(const ContentVerifierTest&) = delete;
ContentVerifierTest& operator=(const ContentVerifierTest&) = delete;
void SetUp() override { void SetUp() override {
ExtensionsTest::SetUp(); ExtensionsTest::SetUp();
background_manifest_type_ = GetParam();
// Manually register handlers since the |ContentScriptsHandler| is not // Manually register handlers since the |ContentScriptsHandler| is not
// usually registered in extensions_unittests. // usually registered in extensions_unittests.
...@@ -116,16 +116,18 @@ class ContentVerifierTest ...@@ -116,16 +116,18 @@ class ContentVerifierTest
} }
bool ShouldVerifySinglePath(const base::FilePath& path) { bool ShouldVerifySinglePath(const base::FilePath& path) {
std::set<base::FilePath> paths_to_verify; return content_verifier_->ShouldVerifyAnyPathsForTesting(
paths_to_verify.insert(path); extension_->id(), extension_->path(), {path});
return content_verifier_->ShouldVerifyAnyPaths(
extension_->id(), extension_->path(), paths_to_verify);
} }
BackgroundManifestType GetBackgroundManifestType() { BackgroundManifestType GetBackgroundManifestType() {
return background_manifest_type_; return background_manifest_type_;
} }
protected:
BackgroundManifestType background_manifest_type_ =
BackgroundManifestType::kNone;
private: private:
// Create a test extension with a content script and possibly a background // Create a test extension with a content script and possibly a background
// page or background script. // page or background script.
...@@ -167,17 +169,28 @@ class ContentVerifierTest ...@@ -167,17 +169,28 @@ class ContentVerifierTest
return extension; return extension;
} }
BackgroundManifestType background_manifest_type_;
scoped_refptr<ContentVerifier> content_verifier_; scoped_refptr<ContentVerifier> content_verifier_;
scoped_refptr<Extension> extension_; scoped_refptr<Extension> extension_;
TestContentVerifierDelegate* content_verifier_delegate_raw_; TestContentVerifierDelegate* content_verifier_delegate_raw_;
};
class ContentVerifierTestWithBackgroundType
: public ContentVerifierTest,
public testing::WithParamInterface<BackgroundManifestType> {
public:
ContentVerifierTestWithBackgroundType() {
background_manifest_type_ = GetParam();
}
DISALLOW_COPY_AND_ASSIGN(ContentVerifierTest); ContentVerifierTestWithBackgroundType(
const ContentVerifierTestWithBackgroundType&) = delete;
ContentVerifierTestWithBackgroundType& operator=(
const ContentVerifierTestWithBackgroundType&) = delete;
}; };
// Verifies that |ContentVerifier::ShouldVerifyAnyPaths| returns true for // Verifies that |ContentVerifier::ShouldVerifyAnyPaths| returns true for
// some file paths even if those paths are specified as browser images. // some file paths even if those paths are specified as browser images.
TEST_P(ContentVerifierTest, BrowserImagesShouldBeVerified) { TEST_P(ContentVerifierTestWithBackgroundType, BrowserImagesShouldBeVerified) {
std::set<base::FilePath> files_to_be_verified = { std::set<base::FilePath> files_to_be_verified = {
kContentScriptPath, kScriptFilePath, kHTMLFilePath, kHTMFilePath}; kContentScriptPath, kScriptFilePath, kHTMLFilePath, kHTMFilePath};
std::set<base::FilePath> files_not_to_be_verified{kIconPath, std::set<base::FilePath> files_not_to_be_verified{kIconPath,
...@@ -213,12 +226,12 @@ TEST_P(ContentVerifierTest, BrowserImagesShouldBeVerified) { ...@@ -213,12 +226,12 @@ TEST_P(ContentVerifierTest, BrowserImagesShouldBeVerified) {
INSTANTIATE_TEST_SUITE_P( INSTANTIATE_TEST_SUITE_P(
All, All,
ContentVerifierTest, ContentVerifierTestWithBackgroundType,
testing::Values(BackgroundManifestType::kNone, testing::Values(BackgroundManifestType::kNone,
BackgroundManifestType::kBackgroundScript, BackgroundManifestType::kBackgroundScript,
BackgroundManifestType::kBackgroundPage)); BackgroundManifestType::kBackgroundPage));
TEST(ContentVerifierTest, NormalizeRelativePath) { TEST_F(ContentVerifierTest, NormalizeRelativePath) {
// This macro helps avoid wrapped lines in the test structs. // This macro helps avoid wrapped lines in the test structs.
#define FPL(x) FILE_PATH_LITERAL(x) #define FPL(x) FILE_PATH_LITERAL(x)
struct TestData { struct TestData {
...@@ -238,4 +251,25 @@ TEST(ContentVerifierTest, NormalizeRelativePath) { ...@@ -238,4 +251,25 @@ TEST(ContentVerifierTest, NormalizeRelativePath) {
} }
} }
// Tests that JavaScript and html/htm files are always verified, even if their
// extension case isn't lower cased or even if they are specified as browser
// image paths.
TEST_F(ContentVerifierTest, JSAndHTMLAlwaysVerified) {
std::vector<std::string> paths = {
"a.js", "b.html", "c.htm", "a.JS", "b.HTML",
"c.HTM", "a.Js", "b.Html", "c.Htm",
};
for (const auto& path_str : paths) {
const base::FilePath path = base::FilePath().AppendASCII(path_str);
UpdateBrowserImagePaths({});
// |path| would be treated as unclassified resource, so it gets verified.
EXPECT_TRUE(ShouldVerifySinglePath(path)) << "for path " << path;
// Even if |path| was specified as browser image, as |path| is JS/html
// (sensitive) resource, it would still get verified.
UpdateBrowserImagePaths({path});
EXPECT_TRUE(ShouldVerifySinglePath(path)) << "for path " << path;
}
}
} // namespace extensions } // namespace extensions
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment