From d55570905a1471af7eed00c9ee633af06f68595f Mon Sep 17 00:00:00 2001
From: Jilin Yang <[email protected]>
Date: Fri, 29 May 2026 14:40:05 -0700
Subject: [PATCH] Escape async query param in OneGoogleBar URL construction

GetApiUrl() concatenated the render-supplied "async" value into the
request query unescaped, allowing query-parameter injection into the
credentialed newtab_ogb request. Escape it with EscapeQueryParamValue,
restoring only the ':' and ',' then backend needs literal.

Fixed: b:517585486
Change-Id: I61d6b8e519f2651dc89f42fbb0463b2adfdaf796
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7880156
Reviewed-by: Riley Tatum <[email protected]>
Commit-Queue: Jilin Yang <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1638742}
---

diff --git a/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl.cc b/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl.cc
index a2ee72c..bfd5f86d 100644
--- a/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl.cc
+++ b/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl.cc
@@ -13,6 +13,7 @@
 #include "base/functional/bind.h"
 #include "base/functional/callback.h"
 #include "base/json/json_writer.h"
+#include "base/strings/escape.h"
 #include "base/strings/string_util.h"
 #include "base/values.h"
 #include "build/build_config.h"
@@ -336,13 +337,16 @@
   }
 
   for (const auto& param_pair : additional_query_params_) {
-    // Add the "async=" parameter. We can't use net::AppendQueryParameter for
-    // this because we need the ":" to remain unescaped.
+    // Escape to neutralize injection chars (e.g., '&', '='), then restore the
+    // ':' and ',' that the backend requires to stay literal in "async".
     if (param_pair.first == "async") {
-      std::string query = api_url.GetQuery() + "&async=" + param_pair.second;
-      if (query.at(0) == '&') {
-        query = query.substr(1);
-      }
+      std::string async_value =
+          base::EscapeQueryParamValue(param_pair.second, /*use_plus*/ true);
+      base::ReplaceSubstringsAfterOffset(&async_value, 0, "%2C", ",");
+      base::ReplaceSubstringsAfterOffset(&async_value, 0, "%3A", ":");
+      std::string query = api_url.GetQuery();
+      query = query.empty() ? "async=" + async_value
+                            : query + "&async=" + async_value;
       GURL::Replacements replacements;
       replacements.SetQueryStr(query);
       api_url = api_url.ReplaceComponents(replacements);
diff --git a/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl_unittest.cc b/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl_unittest.cc
index 0c1f88ee..305527e8 100644
--- a/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl_unittest.cc
+++ b/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl_unittest.cc
@@ -21,6 +21,7 @@
 #include "components/signin/core/browser/signin_header_helper.h"
 #include "components/variations/scoped_variations_ids_provider.h"
 #include "content/public/test/browser_task_environment.h"
+#include "net/base/url_util.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_status_code.h"
 #include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
@@ -182,6 +183,36 @@
             one_google_bar_loader()->GetLoadURLForTesting().GetQuery());
 }
 
+TEST_F(OneGoogleBarLoaderImplTest, AsyncParamInjectionIsNeutralized) {
+  one_google_bar_loader()->SetAdditionalQueryParams(
+      {{"hl", ""}, {"async", "fixed:0&authuser=1&INJECTED_PARAM=1"}});
+
+  GURL url = one_google_bar_loader()->GetLoadURLForTesting();
+
+  std::string value;
+  EXPECT_FALSE(net::GetValueForKeyInQuery(url, "authuser", &value));
+  EXPECT_FALSE(net::GetValueForKeyInQuery(url, "INJECTED_PARAM", &value));
+
+  EXPECT_EQ("async=fixed:0%26authuser%3D1%26INJECTED_PARAM%3D1&hl=",
+            url.GetQuery());
+}
+
+TEST_F(OneGoogleBarLoaderImplTest, AsyncParamPreservesColonAndComma) {
+  one_google_bar_loader()->SetAdditionalQueryParams(
+      {{"hl", ""}, {"async", "fixed:0,abp:1"}});
+
+  EXPECT_EQ("async=fixed:0,abp:1&hl=",
+            one_google_bar_loader()->GetLoadURLForTesting().GetQuery());
+}
+
+TEST_F(OneGoogleBarLoaderImplTest, NonAsyncParamColonStaysEscaped) {
+  one_google_bar_loader()->SetAdditionalQueryParams(
+      {{"hl", ""}, {"async", "fixed:0"}, {"foo", "a:b"}});
+
+  EXPECT_EQ("async=fixed:0&foo=a%3Ab&hl=",
+            one_google_bar_loader()->GetLoadURLForTesting().GetQuery());
+}
+
 TEST_F(OneGoogleBarLoaderImplTest, RequestReturns) {
   SetUpResponseWithData(kMinimalValidResponse);
 

diff --git a/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl_unittest.cc b/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl_unittest.cc
index 0c1f88ee..305527e8 100644
--- a/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl_unittest.cc
+++ b/chrome/browser/new_tab_page/one_google_bar/one_google_bar_loader_impl_unittest.cc
@@ -21,6 +21,7 @@
 #include "components/signin/core/browser/signin_header_helper.h"
 #include "components/variations/scoped_variations_ids_provider.h"
 #include "content/public/test/browser_task_environment.h"
+#include "net/base/url_util.h"
 #include "net/http/http_request_headers.h"
 #include "net/http/http_status_code.h"
 #include "services/data_decoder/public/cpp/test_support/in_process_data_decoder.h"
@@ -182,6 +183,36 @@
             one_google_bar_loader()->GetLoadURLForTesting().GetQuery());
 }
 
+TEST_F(OneGoogleBarLoaderImplTest, AsyncParamInjectionIsNeutralized) {
+  one_google_bar_loader()->SetAdditionalQueryParams(
+      {{"hl", ""}, {"async", "fixed:0&authuser=1&INJECTED_PARAM=1"}});
+
+  GURL url = one_google_bar_loader()->GetLoadURLForTesting();
+
+  std::string value;
+  EXPECT_FALSE(net::GetValueForKeyInQuery(url, "authuser", &value));
+  EXPECT_FALSE(net::GetValueForKeyInQuery(url, "INJECTED_PARAM", &value));
+
+  EXPECT_EQ("async=fixed:0%26authuser%3D1%26INJECTED_PARAM%3D1&hl=",
+            url.GetQuery());
+}
+
+TEST_F(OneGoogleBarLoaderImplTest, AsyncParamPreservesColonAndComma) {
+  one_google_bar_loader()->SetAdditionalQueryParams(
+      {{"hl", ""}, {"async", "fixed:0,abp:1"}});
+
+  EXPECT_EQ("async=fixed:0,abp:1&hl=",
+            one_google_bar_loader()->GetLoadURLForTesting().GetQuery());
+}
+
+TEST_F(OneGoogleBarLoaderImplTest, NonAsyncParamColonStaysEscaped) {
+  one_google_bar_loader()->SetAdditionalQueryParams(
+      {{"hl", ""}, {"async", "fixed:0"}, {"foo", "a:b"}});
+
+  EXPECT_EQ("async=fixed:0&foo=a%3Ab&hl=",
+            one_google_bar_loader()->GetLoadURLForTesting().GetQuery());
+}
+
 TEST_F(OneGoogleBarLoaderImplTest, RequestReturns) {
   SetUpResponseWithData(kMinimalValidResponse);