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 <jdonnelly@chromium.org>
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 <ananyaseelam@google.com>
Commit-Queue: Justin Donnelly <jdonnelly@chromium.org>
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": "badEmail1@gmail.com"}, {"emailAddress": "gOOdemaIl@gmail.com"}]}}},
- {"title": "bad title1 title2", "score": 997, "url": "url", "metadata": {"owner": {"emailAddresses": [{"emailAddress": "badEmail2@gmail.com"}]}}},
- {"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": "badEmail1@gmail.com"},
+ {"emailAddress": "gOOdemaIl@gmail.com"}
+ ]
+ }
+ }
+ },
+ {
+ "title": "bad title1 title2",
+ "score": 997,
+ "url": "https://documentprovider.tld/doc?id=4",
+ "metadata": {
+ "owner": {
+ "emailAddresses": [
+ {"emailAddress": "badEmail2@gmail.com"}
+ ]
+ }
+ }
+ },
+ {
+ "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