Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

css: Use document (not base) URL for inline style preloads' referrers #29940

Merged
merged 1 commit into from Sep 23, 2021

Conversation

chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Aug 7, 2021

crrev.com/c/2592447 fixed one code path where setting a document's base
URL (via the HTML <base> tag) led to requests from inline CSS using the
base URL as their referrer, rather than the document URL. This goes
against the recommendation in the Referrer Policy spec that requests
from inline CSS use their documents' referrers. [1] In general, we try
to avoid letting pages override outgoing requests' referrers to
different-origin URLs, even though this is not a hard security boundary.

It turns out a separate code path can also trigger requests from inline
style sheets: in particular, '@import' statements in inline stylesheets
get prefetched by the HTML parser, which currently has separate logic
that explicitly sets those requests' referrers to the document's base
URL.

This change removes that logic. After this change, preload requests from
inline style in the HTML parser will use the document's URL, not its
base URL, when generating their referrers. This CL also adds two new WPTs:

  • "stylesheet-with-differentorigin-base-url.html" verifies the referrer
    for an inline stylesheet requesting another stylesheet via an @import
    statement. There are other tests inspecting the referrers for SVG and
    image fetches from inline stylesheets, but not for child stylesheet
    fetches. This test passes even without this CL applied (because of
    crrev.com/c/2592447).
  • "stylesheet-with-differentorigin-base-url-from-preload.html" does the
    same thing, except from a srcdoc iframe. Using a srcdoc iframe triggers
    the preload code path since the inline stylesheet is hardcoded in a
    <style> HTML tag. (In contrast, the test above uses JS to add the style
    element to the DOM.) Because this second test exercises the preload
    codepath, it fails without this patch's functional changes applied.

With this patch applied, the repro in the linked bug no longer succeeds.

[1] https://www.w3.org/TR/referrer-policy/#integration-with-css

Test: New WPT covers the preload path. Manually tested the bug's repro.
Change-Id: I6bd797978b207a4bc0bb1b35565eb93c7162729f
Fixed: 1233375
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3078937
Commit-Queue: David Van Cleve <davidvc@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#924146}

Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The review process for this patch is being conducted in the Chromium project.

@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-3078937 branch 2 times, most recently from 6f3c39a to f3326ef Compare September 8, 2021 00:22
@chromium-wpt-export-bot chromium-wpt-export-bot force-pushed the chromium-export-cl-3078937 branch 3 times, most recently from c2bbae1 to 3594e68 Compare September 23, 2021 00:54
crrev.com/c/2592447 fixed one code path where setting a document's base
URL (via the HTML <base> tag) led to requests from inline CSS using the
base URL as their referrer, rather than the document URL. This goes
against the recommendation in the Referrer Policy spec that requests
from inline CSS use their documents' referrers. [1] In general, we try
to avoid letting pages override outgoing requests' referrers to
different-origin URLs, even though this is not a hard security boundary.

It turns out a separate code path can also trigger requests from inline
style sheets: in particular, '@import' statements in inline stylesheets
get prefetched by the HTML parser, which currently has separate logic
that explicitly sets those requests' referrers to the document's base
URL.

This change removes that logic. After this change, preload requests from
inline style in the HTML parser will use the document's URL, not its
base URL, when generating their referrers. This CL also adds two new WPTs:
* "stylesheet-with-differentorigin-base-url.html" verifies the referrer
for an inline stylesheet requesting another stylesheet via an @import
statement. There are other tests inspecting the referrers for SVG and
image fetches from inline stylesheets, but not for child stylesheet
fetches. This test passes even without this CL applied (because of
crrev.com/c/2592447).
* "stylesheet-with-differentorigin-base-url-from-preload.html" does the
same thing, except from a srcdoc iframe. Using a srcdoc iframe triggers
the preload code path since the inline stylesheet is hardcoded in a
<style> HTML tag. (In contrast, the test above uses JS to add the style
element to the DOM.) Because this second test exercises the preload
codepath, it fails without this patch's functional changes applied.

With this patch applied, the repro in the linked bug no longer succeeds.

[1] https://www.w3.org/TR/referrer-policy/#integration-with-css

Test: New WPT covers the preload path. Manually tested the bug's repro.
Change-Id: I6bd797978b207a4bc0bb1b35565eb93c7162729f
Fixed: 1233375
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/3078937
Commit-Queue: David Van Cleve <davidvc@chromium.org>
Reviewed-by: Jochen Eisinger <jochen@chromium.org>
Reviewed-by: Emily Stark <estark@chromium.org>
Reviewed-by: Yoav Weiss <yoavweiss@chromium.org>
Cr-Commit-Position: refs/heads/main@{#924146}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants