diff --git a/content/services/auction_worklet/bidder_worklet_unittest.cc b/content/services/auction_worklet/bidder_worklet_unittest.cc index d8e41497ec06f3..ca23de2369e567 100644 --- a/content/services/auction_worklet/bidder_worklet_unittest.cc +++ b/content/services/auction_worklet/bidder_worklet_unittest.cc @@ -9098,8 +9098,10 @@ TEST_F(BidderWorkletTest, class BidderWorkletAdMacroReportingEnabledTest : public BidderWorkletTest { public: BidderWorkletAdMacroReportingEnabledTest() { - scoped_feature_list_.InitAndEnableFeature( - blink::features::kAdAuctionReportingWithMacroApi); + scoped_feature_list_.InitWithFeatures( + /*enabled_features=*/{blink::features::kAdAuctionReportingWithMacroApi, + blink::features::kFencedFramesM119Features}, + /*disabled_features=*/{}); } private: @@ -9150,6 +9152,18 @@ TEST_F(BidderWorkletAdMacroReportingEnabledTest, ReportWinRegisterAdMacro) { /*expected_ad_beacon_map=*/{}, {{"uppercase", "ABC"}, {"lowercase", "abc"}}); + // URL-encoded strings should be accepted. + RunReportWinWithFunctionBodyExpectingResult( + R"(registerAdMacro('URL_ENC_KEY', 'http%3A%2F%2Fpub%2Eexample%2Fpage'); + registerAdMacro('http%3A%2F%2Fpub%2Eexample%2Fpage', 'URL_ENC_VAL'); + registerAdMacro('URL_ENC_KEY_http%3A%2F', 'URL_ENC_VAL_http%3A%2F'); + )", + /*expected_report_url=*/absl::nullopt, + /*expected_ad_beacon_map=*/{}, + {{"URL_ENC_KEY", "http%3A%2F%2Fpub%2Eexample%2Fpage"}, + {"http%3A%2F%2Fpub%2Eexample%2Fpage", "URL_ENC_VAL"}, + {"URL_ENC_KEY_http%3A%2F", "URL_ENC_VAL_http%3A%2F"}}); + // When called multiple times for a macro name, use the last valid call's // value. RunReportWinWithFunctionBodyExpectingResult( @@ -9193,6 +9207,14 @@ TEST_F(BidderWorkletAdMacroReportingEnabledTest, {R"(registerAdMacro('123', {toString:{}});)", "https://url.test/:11 Uncaught TypeError: Cannot convert object to " "primitive value."}, + // Invalid characters in macro key. + {R"(registerAdMacro('}${FOO', 'foo');)", + "https://url.test/:11 Uncaught TypeError: registerAdMacro macro key and " + "value must be URL-encoded."}, + // Invalid characters in macro value. + {R"(registerAdMacro('MACRO_KEY', 'baz&foo=bar');)", + "https://url.test/:11 Uncaught TypeError: registerAdMacro macro key and " + "value must be URL-encoded."}, }; for (const auto& test_case : kTestCases) { RunReportWinWithFunctionBodyExpectingResult( diff --git a/content/services/auction_worklet/register_ad_macro_bindings.cc b/content/services/auction_worklet/register_ad_macro_bindings.cc index 50551084f5d678..fae70ebb8f7054 100644 --- a/content/services/auction_worklet/register_ad_macro_bindings.cc +++ b/content/services/auction_worklet/register_ad_macro_bindings.cc @@ -14,6 +14,7 @@ #include "content/services/auction_worklet/public/mojom/bidder_worklet.mojom.h" #include "content/services/auction_worklet/webidl_compat.h" #include "third_party/blink/public/common/features.h" +#include "url/url_util.h" #include "v8/include/v8-exception.h" #include "v8/include/v8-external.h" #include "v8/include/v8-function-callback.h" @@ -67,6 +68,21 @@ void RegisterAdMacroBindings::RegisterAdMacro( return; } + auto ContainsDisallowedCharacters = [](const std::string& str) -> bool { + return base::ranges::any_of( + str, [](char c) { return !url::IsURIComponentChar(c) && c != '%'; }); + }; + + if (base::FeatureList::IsEnabled( + blink::features::kFencedFramesM119Features)) { + if (ContainsDisallowedCharacters(macro_name) || + ContainsDisallowedCharacters(macro_value)) { + args.GetIsolate()->ThrowException( + v8::Exception::TypeError(v8_helper->CreateStringFromLiteral( + "registerAdMacro macro key and value must be URL-encoded"))); + } + } + bindings->ad_macro_map_[macro_name] = macro_value; } diff --git a/third_party/blink/common/features.cc b/third_party/blink/common/features.cc index 1715c2e1f586b0..6a3685454987f7 100644 --- a/third_party/blink/common/features.cc +++ b/third_party/blink/common/features.cc @@ -760,6 +760,8 @@ BASE_FEATURE(kFencedFrames, "FencedFrames", base::FEATURE_DISABLED_BY_DEFAULT); // * Extra format for ad size macro substitution: // ${AD_WIDTH} and ${AD_HEIGHT}, on top of the previous // {%AD_WIDTH%} and {%AD_HEIGHT%}. +// * Input validation (no disallowed URI component characters) in +// registerAdMacro keys and values. BASE_FEATURE(kFencedFramesM119Features, "FencedFramesM119Features", base::FEATURE_DISABLED_BY_DEFAULT); diff --git a/url/url_util.cc b/url/url_util.cc index 001c50e72fd244..d624baae72884e 100644 --- a/url/url_util.cc +++ b/url/url_util.cc @@ -918,6 +918,10 @@ void EncodeURIComponent(const char* input, int length, CanonOutput* output) { } } +bool IsURIComponentChar(char c) { + return IsComponentChar(c); +} + bool CompareSchemeComponent(const char* spec, const Component& component, const char* compare_to) { diff --git a/url/url_util.h b/url/url_util.h index 670552a8ce12ad..d1e101c2f31daf 100644 --- a/url/url_util.h +++ b/url/url_util.h @@ -309,6 +309,13 @@ void DecodeURLEscapeSequences(const char* input, COMPONENT_EXPORT(URL) void EncodeURIComponent(const char* input, int length, CanonOutput* output); +// Returns true if `c` is a character that does not require escaping in +// encodeURIComponent. +// TODO(crbug.com/1481056): Remove this when event-level reportEvent is removed +// (if it is still this function's only consumer). +COMPONENT_EXPORT(URL) +bool IsURIComponentChar(char c); + } // namespace url #endif // URL_URL_UTIL_H_