Commit 648127e0 authored by manukh's avatar manukh Committed by Chromium LUCI CQ

[omnibox] Truncate |TemplateUrl::TryEncoding()| input to 1,000,000 chars

|TryEncoding()| invokes |base::UTF16ToCodepage()|,
|net::EscapeQueryParamValue()|, and |net::EscapePath()|. All 3 create
strings of 3 times the length of the input. This is suspected to be
causing about 50 crashes per month.

Invocations to |TryEncoding()| that have resulted in crashes have
originated from different sources; e.g. omnibox & autocomplete,
extensions, and on startup.

This CL truncates the input string to 1,000,000 chars.

Bug: 1137686
Change-Id: I16e997666c07611d24472bcbee99893138e9c5e8
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2559284
Commit-Queue: manuk hovanesian <manukh@chromium.org>
Reviewed-by: default avatarTommy Li <tommycli@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831913}
parent 20d88052
...@@ -72,6 +72,8 @@ const char kDefaultCount[] = "10"; ...@@ -72,6 +72,8 @@ const char kDefaultCount[] = "10";
// Used if the output encoding parameter is required. // Used if the output encoding parameter is required.
const char kOutputEncodingType[] = "UTF-8"; const char kOutputEncodingType[] = "UTF-8";
const size_t kMaxStringEncodeStringLength = 1'000'000;
// Attempts to encode |terms| and |original_query| in |encoding| and escape // Attempts to encode |terms| and |original_query| in |encoding| and escape
// them. |terms| may be escaped as path or query depending on |is_in_query|; // them. |terms| may be escaped as path or query depending on |is_in_query|;
// |original_query| is always escaped as query. If |force_encode| is true // |original_query| is always escaped as query. If |force_encode| is true
...@@ -86,12 +88,23 @@ bool TryEncoding(const base::string16& terms, ...@@ -86,12 +88,23 @@ bool TryEncoding(const base::string16& terms,
base::string16* escaped_original_query) { base::string16* escaped_original_query) {
DCHECK(escaped_terms); DCHECK(escaped_terms);
DCHECK(escaped_original_query); DCHECK(escaped_original_query);
// Both |base::UTF16ToCodepage()| and |net::Escape*()| invocations below
// create strings longer than their inputs. To ensure doing so does not crash,
// this truncates |terms| to |kMaxStringEncodeStringLength|.
const base::string16& truncated_terms =
terms.size() > kMaxStringEncodeStringLength
? terms.substr(0, kMaxStringEncodeStringLength)
: terms;
base::OnStringConversionError::Type error_handling = base::OnStringConversionError::Type error_handling =
force_encode ? base::OnStringConversionError::SKIP force_encode ? base::OnStringConversionError::SKIP
: base::OnStringConversionError::FAIL; : base::OnStringConversionError::FAIL;
std::string encoded_terms; std::string encoded_terms;
if (!base::UTF16ToCodepage(terms, encoding, error_handling, &encoded_terms)) if (!base::UTF16ToCodepage(truncated_terms, encoding, error_handling,
&encoded_terms)) {
return false; return false;
}
*escaped_terms = base::UTF8ToUTF16(is_in_query ? *escaped_terms = base::UTF8ToUTF16(is_in_query ?
net::EscapeQueryParamValue(encoded_terms, true) : net::EscapeQueryParamValue(encoded_terms, true) :
net::EscapePath(encoded_terms)); net::EscapePath(encoded_terms));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment