Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Mitigate scriptlet injection races (uplift to 1.30.x) #10199

Merged
merged 1 commit into from
Sep 23, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 13 additions & 10 deletions components/cosmetic_filters/browser/cosmetic_filters_resources.cc
Original file line number Diff line number Diff line change
Expand Up @@ -80,32 +80,35 @@ void CosmeticFiltersResources::HiddenClassIdSelectorsOnUI(
}

void CosmeticFiltersResources::UrlCosmeticResourcesOnUI(
UrlCosmeticResourcesCallback callback,
base::OnceCallback<void(base::Value)> callback,
absl::optional<base::Value> resources) {
std::move(callback).Run(resources ? std::move(resources.value())
: base::Value());
}

void CosmeticFiltersResources::ShouldDoCosmeticFiltering(
void CosmeticFiltersResources::UrlCosmeticResources(
const std::string& url,
ShouldDoCosmeticFilteringCallback callback) {
UrlCosmeticResourcesCallback callback) {
bool enabled =
brave_shields::ShouldDoCosmeticFiltering(settings_map_, GURL(url));

if (!enabled) {
std::move(callback).Run(enabled, false, base::Value());
return;
}

bool first_party_enabled =
brave_shields::IsFirstPartyCosmeticFilteringEnabled(settings_map_,
GURL(url));
std::move(callback).Run(enabled, first_party_enabled);
}

void CosmeticFiltersResources::UrlCosmeticResources(
const std::string& url,
UrlCosmeticResourcesCallback callback) {
ad_block_service_->GetTaskRunner()->PostTaskAndReplyWithResult(
FROM_HERE,
base::BindOnce(&brave_shields::AdBlockService::UrlCosmeticResources,
base::Unretained(ad_block_service_), url),
base::BindOnce(&CosmeticFiltersResources::UrlCosmeticResourcesOnUI,
weak_factory_.GetWeakPtr(), std::move(callback)));
base::BindOnce(
&CosmeticFiltersResources::UrlCosmeticResourcesOnUI,
weak_factory_.GetWeakPtr(),
base::BindOnce(std::move(callback), enabled, first_party_enabled)));
}

} // namespace cosmetic_filters
16 changes: 7 additions & 9 deletions components/cosmetic_filters/browser/cosmetic_filters_resources.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include <string>
#include <vector>

#include "base/callback.h"
#include "base/memory/weak_ptr.h"
#include "base/values.h"
#include "brave/components/cosmetic_filters/common/cosmetic_filters.mojom.h"
Expand All @@ -35,28 +36,25 @@ class CosmeticFiltersResources final
brave_shields::AdBlockService* ad_block_service);
~CosmeticFiltersResources() override;

// Sends back to renderer a response: do we need to apply cosmetic filters
// for the url.
void ShouldDoCosmeticFiltering(
const std::string& url,
ShouldDoCosmeticFilteringCallback callback) override;

// Sends back to renderer a response about rules that has to be applied
// for the specified selectors.
void HiddenClassIdSelectors(const std::string& input,
const std::vector<std::string>& exceptions,
HiddenClassIdSelectorsCallback callback) override;

// Sends back to renderer a response what rules and scripts has to be
// applied for the specified url.
// If cosmetic filtering is enabled, sends the renderer a response including
// whether or not to apply cosmetic filtering to first party elements along
// with an initial set of rules and scripts to apply for the given URL.
// If cosmetic filtering is disabled, `enabled` will be set to false and the
// other return values should be ignored.
void UrlCosmeticResources(const std::string& url,
UrlCosmeticResourcesCallback callback) override;

private:
void HiddenClassIdSelectorsOnUI(HiddenClassIdSelectorsCallback callback,
absl::optional<base::Value> resources);

void UrlCosmeticResourcesOnUI(UrlCosmeticResourcesCallback callback,
void UrlCosmeticResourcesOnUI(base::OnceCallback<void(base::Value)> callback,
absl::optional<base::Value> resources);

HostContentSettingsMap* settings_map_; // Not owned
Expand Down
7 changes: 4 additions & 3 deletions components/cosmetic_filters/common/cosmetic_filters.mojom
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,11 @@ module cosmetic_filters.mojom;
import "mojo/public/mojom/base/values.mojom";

interface CosmeticFiltersResources {
ShouldDoCosmeticFiltering(string url) => (bool enabled,
bool first_party_enabled);
UrlCosmeticResources(string url) => (mojo_base.mojom.Value result);
// Receives an input string which is JSON object.
HiddenClassIdSelectors(string input, array<string> exceptions) => (
mojo_base.mojom.Value result);

UrlCosmeticResources(string url) => (bool enabled,
bool first_party_enabled,
mojo_base.mojom.Value result);
};
Original file line number Diff line number Diff line change
Expand Up @@ -292,20 +292,6 @@ void CosmeticFiltersJSHandler::ProcessURL(const GURL& url,
if (!EnsureConnected() || url_.is_empty() || !url_.is_valid())
return;

cosmetic_filters_resources_->ShouldDoCosmeticFiltering(
url_.spec(),
base::BindOnce(&CosmeticFiltersJSHandler::OnShouldDoCosmeticFiltering,
base::Unretained(this), std::move(callback)));
}

void CosmeticFiltersJSHandler::OnShouldDoCosmeticFiltering(
base::OnceClosure callback,
bool enabled,
bool first_party_enabled) {
if (!enabled || !EnsureConnected())
return;

enabled_1st_party_cf_ = first_party_enabled;
cosmetic_filters_resources_->UrlCosmeticResources(
url_.spec(),
base::BindOnce(&CosmeticFiltersJSHandler::OnUrlCosmeticResources,
Expand All @@ -314,7 +300,12 @@ void CosmeticFiltersJSHandler::OnShouldDoCosmeticFiltering(

void CosmeticFiltersJSHandler::OnUrlCosmeticResources(
base::OnceClosure callback,
bool enabled,
bool first_party_enabled,
base::Value result) {
if (!enabled || !EnsureConnected())
return;
enabled_1st_party_cf_ = first_party_enabled;
resources_dict_ = base::DictionaryValue::From(
base::Value::ToUniquePtrValue(std::move(result)));
std::move(callback).Run();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,10 @@ class CosmeticFiltersJSHandler {
// A function to be called from JS
void HiddenClassIdSelectors(const std::string& input);

void OnShouldDoCosmeticFiltering(base::OnceClosure callback,
bool enabled,
bool first_party_enabled);
void OnUrlCosmeticResources(base::OnceClosure callback, base::Value result);
void OnUrlCosmeticResources(base::OnceClosure callback,
bool enabled,
bool first_party_enabled,
base::Value result);
void CSSRulesRoutine(base::DictionaryValue* resources_dict);
void OnHiddenClassIdSelectors(base::Value result);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -84,9 +84,14 @@ void CosmeticFiltersJsRenderFrameObserver::ReadyToCommitNavigation(
}

void CosmeticFiltersJsRenderFrameObserver::RunScriptsAtDocumentStart() {
ready_->Post(FROM_HERE,
base::BindOnce(&CosmeticFiltersJsRenderFrameObserver::ApplyRules,
weak_factory_.GetWeakPtr()));
if (ready_->is_signaled()) {
ApplyRules();
} else {
ready_->Post(
FROM_HERE,
base::BindOnce(&CosmeticFiltersJsRenderFrameObserver::ApplyRules,
weak_factory_.GetWeakPtr()));
}
}

void CosmeticFiltersJsRenderFrameObserver::ApplyRules() {
Expand Down