Chrome · Omnibox
CVE-2026-84357
Logic Error in Omnibox
Overview
High
Severity
—
CVSS
No
Exploited ITW
Fixed
Fix Status
Changed Functions
| Function | Change | Notes |
|---|---|---|
forcomponents/omnibox/browser/document_provider_unittest.cc |
modified |
Files Changed
components/omnibox/browser/document_provider.cccomponents/omnibox/browser/document_provider_unittest.cc
Patch
From 83357115666f96501a50d1e7db25a811077da06d Mon Sep 17 00:00:00 2001 From: Justin Donnelly <[email protected]> Date: Wed, 26 Aug 2026 09:04:53 -0700 Subject: [PATCH] Ignore invalid and non-HTTP destination URLs in `DocumentProvider`. Bug: b:523208474 Change-Id: I3e977b67ae6aecba23d30df386b5a7e18d64868c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8290168 Reviewed-by: Ananya Seelam <[email protected]> Commit-Queue: Justin Donnelly <[email protected]> Cr-Commit-Position: refs/heads/main@{#1686444} --- diff --git a/components/omnibox/browser/document_provider.cc b/components/omnibox/browser/document_provider.cc index 7fc9eef..1e49dd1 100644 --- a/components/omnibox/browser/document_provider.cc +++ b/components/omnibox/browser/document_provider.cc @@ -778,12 +778,20 @@ AutocompleteMatch match(this, score, false, AutocompleteMatchType::DOCUMENT_SUGGESTION); + // Only allow valid HTTP or HTTPS URLs. + GURL destination_url = GURL(url); + if (!destination_url.is_valid() || + !destination_url.SchemeIsHTTPOrHTTPS()) { + continue; + } + match.destination_url = destination_url; + // Use full URL for navigation. If present, use "originalUrl" for display & // deduping, as it's shorter. const std::string short_url = FindStringKeyOrFallback(result, "originalUrl", url); match.fill_into_edit = base::UTF8ToUTF16(short_url); - match.destination_url = GURL(url); + // `AutocompleteMatch::GURLToStrippedGURL()` will try to use // `GetURLForDeduping()` to extract a doc ID and generate a canonical doc // URL; this is ideal as it handles different URL formats pointing to the diff --git a/components/omnibox/browser/document_provider_unittest.cc b/components/omnibox/browser/document_provider_unittest.cc index a23f32c3..75fa23a 100644 --- a/components/omnibox/browser/document_provider_unittest.cc +++ b/components/omnibox/browser/document_provider_unittest.cc @@ -35,6 +35,7 @@ #include "components/omnibox/common/omnibox_features.h" #include "components/search_engines/search_engines_test_environment.h" #include "testing/gtest/include/gtest/gtest.h" +#include "url/gurl.h" namespace { @@ -98,7 +99,7 @@ static std::string MakeTestResponse(const std::vector<std::string>& doc_ids, int scores) { std::string results = ""; - for (auto doc_id : doc_ids) + for (auto doc_id : doc_ids) { results += base::StringPrintf( R"({ "title": "Document %s longer title", @@ -107,6 +108,7 @@ "originalUrl": "https://drive.google.com/open?id=%s", },)", doc_id.c_str(), scores, doc_id.c_str(), doc_id.c_str()); + } return base::StringPrintf(R"({"results": [%s]})", results.c_str()); } @@ -356,6 +358,82 @@ u"http://sites.google.com/google.com/abc/def"); } +TEST_F(DocumentProviderTest, + ParseDocumentSearchResultsDiscardNonHttpAndInvalidUrls) { + const std::string kJSONResponse = R"({ + "results": [ + { + "title": "Valid HTTPS Document", + "url": "https://documentprovider.tld/doc?id=1", + "score": 1000 + }, + { + "title": "JavaScript URL", + "url": "javascript:alert(1);", + "score": 900 + }, + { + "title": "Valid HTTP Document", + "url": "http://documentprovider.tld/doc?id=2", + "score": 800 + }, + { + "title": "Invalid URL Not A URL", + "url": "not a valid url", + "score": 700 + }, + { + "title": "Data URL", + "url": "data:text/html,Hello", + "score": 600 + }, + { + "title": "FTP URL", + "url": "ftp://example.com/file", + "score": 500 + }, + { + "title": "File URL", + "url": "file:///path/to/file", + "score": 400 + }, + { + "title": "Invalid URL Bad Scheme", + "url": "http:://google.com", + "score": 300 + }, + { + "title": "Another Valid HTTPS Document", + "url": "https://documentprovider.tld/doc?id=3", + "score": 200 + } + ] + })"; + + std::optional<base::Value> response = base::JSONReader::Read( + kJSONResponse, base::JSON_PARSE_CHROMIUM_EXTENSIONS); + ASSERT_TRUE(response); + ASSERT_TRUE(response->is_dict()); + + provider_->input_.UpdateText(u"Document", 0, {}); + ACMatches matches = provider_->ParseDocumentSearchResults(*response); + + // Only the 3 valid HTTP/HTTPS URLs should produce matches. + // All non-HTTP and invalid URLs should be discarded. + ASSERT_EQ(matches.size(), 3u); + EXPECT_EQ(matches[0].destination_url, + GURL("https://documentprovider.tld/doc?id=1")); + EXPECT_EQ(matches[0].contents, u"Valid HTTPS Document"); + + EXPECT_EQ(matches[1].destination_url, + GURL("http://documentprovider.tld/doc?id=2")); + EXPECT_EQ(matches[1].contents, u"Valid HTTP Document"); + + EXPECT_EQ(matches[2].destination_url, + GURL("https://documentprovider.tld/doc?id=3")); + EXPECT_EQ(matches[2].contents, u"Another Valid HTTPS Document"); +} + #if BUILDFLAG(IS_IOS) && BUILDFLAG(USE_BLINK) #define MAYBE_ProductDescriptionStringsAndAccessibleLabels \ DISABLED_ProductDescriptionStringsAndAccessibleLabels @@ -1098,65 +1176,180 @@ ACMatches matches = provider_->ParseDocumentSearchResults(*response); ASSERT_EQ(matches.size(), expected_scores.size()); - for (size_t i = 0; i < matches.size(); i++) + for (size_t i = 0; i < matches.size(); i++) { EXPECT_EQ(matches[i].relevance, expected_scores[i]) << "Match " << i; + } }; { SCOPED_TRACE( "Unowned and non-title matching docs are limited. Title matching docs " "are not limited."); - test(R"({"results": [ - {"title": "bad title1 title2", "score": 1000, "url": "good url isn't sufficient"}, - {"title": "bad title1 title2", "score": 999, "url": "url"}, - {"title": "bad title1 title2", "score": 998, "url": "url"}, - {"title": "goOd tItLE1 title2", "score": 997, "url": "url"}, - {"title": "good title1 title2", "score": 996, "url": "url"}, - {"title": "good title1 title2", "score": 995, "url": "url"}, - {"title": "good title1 title2", "score": 994, "url": "url"} - ]})", - // - 'goo': prefix matches are ok. - // - 'title1': all input terms must be in the title or owner, but not - // all title terms must be in the input (e.g. 'title2'). - // - "goOd tItLE1 title2": Case insensitive. - "gOo Title1", {1000, 0, 0, 997, 996, 995, 994}); + test( + R"({ + "results": [ + { + "title": "bad title1 title2", + "score": 1000, + "url": "https://documentprovider.tld/doc?id=1" + }, + { + "title": "bad title1 title2", + "score": 999, + "url": "https://documentprovider.tld/doc?id=2" + }, + { + "title": "bad title1 title2", + "score": 998, + "url": "https://documentprovider.tld/doc?id=3" + }, + { + "title": "goOd tItLE1 title2", + "score": 997,
Loading diff…
Regression Test / PoC
shipped with the fix
diff --git a/components/omnibox/browser/document_provider_unittest.cc b/components/omnibox/browser/document_provider_unittest.cc
index a23f32c3..75fa23a 100644
--- a/components/omnibox/browser/document_provider_unittest.cc
+++ b/components/omnibox/browser/document_provider_unittest.cc
@@ -35,6 +35,7 @@
#include "components/omnibox/common/omnibox_features.h"
#include "components/search_engines/search_engines_test_environment.h"
#include "testing/gtest/include/gtest/gtest.h"
+#include "url/gurl.h"
namespace {
@@ -98,7 +99,7 @@
static std::string MakeTestResponse(const std::vector<std::string>& doc_ids,
int scores) {
std::string results = "";
- for (auto doc_id : doc_ids)
+ for (auto doc_id : doc_ids) {
results += base::StringPrintf(
R"({
"title": "Document %s longer title",
@@ -107,6 +108,7 @@
"originalUrl": "https://drive.google.com/open?id=%s",
},)",
doc_id.c_str(), scores, doc_id.c_str(), doc_id.c_str());
+ }
return base::StringPrintf(R"({"results": [%s]})", results.c_str());
}
@@ -356,6 +358,82 @@
u"http://sites.google.com/google.com/abc/def");
}
+TEST_F(DocumentProviderTest,
+ ParseDocumentSearchResultsDiscardNonHttpAndInvalidUrls) {
+ const std::string kJSONResponse = R"({
+ "results": [
+ {
+ "title": "Valid HTTPS Document",
+ "url": "https://documentprovider.tld/doc?id=1",
+ "score": 1000
+ },
+ {
+ "title": "JavaScript URL",
+ "url": "javascript:alert(1);",
+ "score": 900
+ },
+ {
+ "title": "Valid HTTP Document",
+ "url": "http://documentprovider.tld/doc?id=2",
+ "score": 800
+ },
+ {
+ "title": "Invalid URL Not A URL",
+ "url": "not a valid url",
+ "score": 700
+ },
+ {
+ "title": "Data URL",
+ "url": "data:text/html,Hello",
+ "score": 600
+ },
+ {
+ "title": "FTP URL",
+ "url": "ftp://example.com/file",
+ "score": 500
+ },
+ {
+ "title": "File URL",
+ "url": "file:///path/to/file",
+ "score": 400
+ },
+ {
+ "title": "Invalid URL Bad Scheme",
+ "url": "http:://google.com",
+ "score": 300
+ },
+ {
+ "title": "Another Valid HTTPS Document",
+ "url": "https://documentprovider.tld/doc?id=3",
+ "score": 200
+ }
+ ]
+ })";
+
+ std::optional<base::Value> response = base::JSONReader::Read(
+ kJSONResponse, base::JSON_PARSE_CHROMIUM_EXTENSIONS);
+ ASSERT_TRUE(response);
+ ASSERT_TRUE(response->is_dict());
+
+ provider_->input_.UpdateText(u"Document", 0, {});
+ ACMatches matches = provider_->ParseDocumentSearchResults(*response);
+
+ // Only the 3 valid HTTP/HTTPS URLs should produce matches.
+ // All non-HTTP and invalid URLs should be discarded.
+ ASSERT_EQ(matches.size(), 3u);
+ EXPECT_EQ(matches[0].destination_url,
+ GURL("https://documentprovider.tld/doc?id=1"));
+ EXPECT_EQ(matches[0].contents, u"Valid HTTPS Document");
+
+ EXPECT_EQ(matches[1].destination_url,
+ GURL("http://documentprovider.tld/doc?id=2"));
+ EXPECT_EQ(matches[1].contents, u"Valid HTTP Document");
+
+ EXPECT_EQ(matches[2].destination_url,
+ GURL("https://documentprovider.tld/doc?id=3"));
+ EXPECT_EQ(matches[2].contents, u"Another Valid HTTPS Document");
+}
+
#if BUILDFLAG(IS_IOS) && BUILDFLAG(USE_BLINK)
#define MAYBE_ProductDescriptionStringsAndAccessibleLabels \
DISABLED_ProductDescriptionStringsAndAccessibleLabels
@@ -1098,65 +1176,180 @@
ACMatches matches = provider_->ParseDocumentSearchResults(*response);
ASSERT_EQ(matches.size(), expected_scores.size());
- for (size_t i = 0; i < matches.size(); i++)
+ for (size_t i = 0; i < matches.size(); i++) {
EXPECT_EQ(matches[i].relevance, expected_scores[i]) << "Match " << i;
+ }
};
{
SCOPED_TRACE(
"Unowned and non-title matching docs are limited. Title matching docs "
"are not limited.");
- test(R"({"results": [
- {"title": "bad title1 title2", "score": 1000, "url": "good url isn't sufficient"},
- {"title": "bad title1 title2", "score": 999, "url": "url"},
- {"title": "bad title1 title2", "score": 998, "url": "url"},
- {"title": "goOd tItLE1 title2", "score": 997, "url": "url"},
- {"title": "good title1 title2", "score": 996, "url": "url"},
- {"title": "good title1 title2", "score": 995, "url": "url"},
- {"title": "good title1 title2", "score": 994, "url": "url"}
- ]})",
- // - 'goo': prefix matches are ok.
- // - 'title1': all input terms must be in the title or owner, but not
- // all title terms must be in the input (e.g. 'title2').
- // - "goOd tItLE1 title2": Case insensitive.
- "gOo Title1", {1000, 0, 0, 997, 996, 995, 994});
+ test(
+ R"({
+ "results": [
+ {
+ "title": "bad title1 title2",
+ "score": 1000,
+ "url": "https://documentprovider.tld/doc?id=1"
+ },
+ {
+ "title": "bad title1 title2",
+ "score": 999,
+ "url": "https://documentprovider.tld/doc?id=2"
+ },
+ {
+ "title": "bad title1 title2",
+ "score": 998,
+ "url": "https://documentprovider.tld/doc?id=3"
+ },
+ {
+ "title": "goOd tItLE1 title2",
+ "score": 997,
+ "url": "https://documentprovider.tld/doc?id=4"
+ },
+ {
+ "title": "good title1 title2",
+ "score": 996,
+ "url": "https://documentprovider.tld/doc?id=5"
+ },
+ {
+ "title": "good title1 title2",
+ "score": 995,
+ "url": "https://documentprovider.tld/doc?id=6"
+ },
+ {
+ "title": "good title1 title2",
+ "score": 994,
+ "url": "https://documentprovider.tld/doc?id=7"
+ }
+ ]
+ })",
+ // - 'goo': prefix matches are ok.
+ // - 'title1': all input terms must be in the title or owner, but not
+ // all title terms must be in the input (e.g. 'title2').
+ // - "goOd tItLE1 title2": Case insensitive.
+ "gOo Title1", {1000, 0, 0, 997, 996, 995, 994});
}
{
SCOPED_TRACE("Owned docs are not limited.");
test(
- R"({"results": [
- {"title": "bad title1 title2", "score": 1000, "url": "good url isn't sufficient"},
- {"title": "bad title1 title2", "score": 999, "url": "url"},
- {"title": "bad title1 title2", "score": 998, "url": "url", "metadata": {"owner": {"emailAddresses": [{"emailAddress": "[email protected]"}, {"emailAddress": "[email protected]"}]}}},
- {"title": "bad title1 title2", "score": 997, "url": "url", "metadata": {"owner": {"emailAddresses": [{"emailAddress": "[email protected]"}]}}},
- {"title": "good title1 title2", "score": 996, "url": "url"},
- {"title": "good title1 title2", "score": 995, "url": "url"},
- {"title": "good title1 title2", "score": 994, "url": "url"}
- ]})",
+ R"({
+ "results": [
+ {
+ "title": "bad title1 title2",
+ "score": 1000,
+ "url": "https://documentprovider.tld/doc?id=1"
+ },
+ {
+ "title": "bad title1 title2",
+ "score": 999,
+ "url": "https://documentprovider.tld/doc?id=2"
+ },
+ {
+ "title": "bad title1 title2",
+ "score": 998,
+ "url": "https://documentprovider.tld/doc?id=3",
+ "metadata": {
+ "owner": {
+ "emailAddresses": [
+ {"emailAddress": "[email protected]"},
+ {"emailAddress": "[email protected]"}
+ ]
+ }
+ }
+ },
+ {
+ "title": "bad title1 title2",
+ "score": 997,
+ "url": "https://documentprovider.tld/doc?id=4",
+ "metadata": {
+ "owner": {
+ "emailAddresses": [
+ {"emailAddress": "[email protected]"}
+ ]
+ }
+ }
+ },
+ {
+ "title": "good title1 title2",
+ "score": 996,
+ "url": "https://documentprovider.tld/doc?id=5"
+ },
+ {
+ "title": "good title1 title2",
+ "score": 995,
+ "url": "https://documentprovider.tld/doc?id=6"
+ },
+ {
+ "title": "good title1 title2",
+ "score": 994,
+ "url": "https://documentprovider.tld/doc?id=7"
+ }
+ ]
+ })",
"goo title1", {1000, 0, 998, 0, 996, 995, 994});
}
{
SCOPED_TRACE("Responses with missing owner don't crash and are limited.");
- test(R"({"results": [
- {"title": "title", "score": 1000, "url": "url", "metadata":
- { "owner": { "emailAddresses": [{}] } }
- },
- {"title": "title", "score": 999, "url": "url", "metadata":
- { "owner": { "emailAddresses": [{}] } }
- },
- {"title": "title", "score": 998, "url": "url", "metadata":
- { "owner": { "emailAddresses": [] } }
- },
- {"title": "title", "score": 997, "url": "url", "metadata":
- { "owner": {} }
- },
- {"title": "title", "score": 996, "url": "url", "metadata": {}},
- {"title": "title", "score": 995, "url": "url"},
- {}
- ]})",
- "input", {1000, 0, 0, 0, 0, 0});
+ test(
+ R"({
+ "results": [
+ {
+ "title": "title",
+ "score": 1000,
+ "url": "https://documentprovider.tld/doc?id=1",
+ "metadata": {
+ "owner": {
+ "emailAddresses": [{}]
+ }
+ }
+ },
+ {
+ "title": "title",
+ "score": 999,
+ "url": "https://documentprovider.tld/doc?id=2",
+ "metadata": {
+ "owner": {
+ "emailAddresses": [{}]
+ }
+ }
+ },
+ {
... (truncated)
Loading diff…
Original Bug Report
The reporter's bug is still restricted on the tracker. Chrome de-restricts security bugs ~30–90 days after the fix ships; a later run will backfill it here.
References
On This Page