mozilla / bedrock

Making mozilla.org awesome, one pebble at a time
https://www.mozilla.org
Mozilla Public License 2.0
1.18k stars 919 forks source link

Add `base-uri` Directive to Content Security Policy #15555

Open robhudson opened 1 week ago

robhudson commented 1 week ago

Description

This issue proposes adding the base-uri directive to our Content Security Policy (CSP) to enhance security by controlling the base URL used for resolving relative URLs in our web application. The base-uri directive restricts where <base> elements can point, helping mitigate certain types of injection attacks and preventing the unintended manipulation of relative URL resolution.

Why Add base-uri?

  1. Mitigates Injection Attacks:

    • Attackers may attempt to inject a malicious <base> element into the HTML document, redirecting relative URLs (e.g., links, resources) to an unauthorized or malicious domain.
  2. Improves Application Integrity:

    • Ensures that all relative URLs resolve to trusted origins, maintaining control over URL resolution behavior.
    • Protects user experience and data by preventing exploitation of relative links.
  3. Aligns with Security Best Practices:

    • Adding base-uri to CSP strengthens the policy against attacks targeting URL resolution, complementing other directives like script-src and form-action.
janbrasna commented 6 days ago

There don't seem to be any <base> elements set visibly from a quick search, and I also don't recall any env settings to inject it for some deployments (e.g. don't see it being used even in test.bedrock.nonprod.webservices.*) so the goal is perhaps to set it to 'none', right?

The public facing site should be fine, question is whether Wagtail doesn't need that for anything, but the only base use I can spot is in targets for opening new windows, so a RO test-drive should surface any violations, but hopefully there would be none. 🤞

stevejalim commented 5 days ago

Just an FYI but <base> appears to be used in Wagtail's internal live preview panel (see this image as an example)

https://github.com/wagtail/wagtail/commit/54597bb65d70fc41f05beb6c711ffed82fcd8dc7

So if we do add it, please can we double-check that in-CMS previews still work fine.

janbrasna commented 4 days ago

Noticed this exact use before and it should have no impact: <base target> is not affected by restricting base href, and its possible vectors were decided to be addressed elsewhere so it seems like no target changes are planned in CSP for the foreseeable future.

robhudson commented 4 days ago

There don't seem to be any elements set visibly from a quick search

There aren't but this also protects if a malicious script were to dynamically add a <base> target to the page affecting any forms or other elements on the page that may use relative addressing.

janbrasna commented 2 days ago

There don't seem to be any elements set visibly from a quick search

There aren't but this also protects if a malicious script were to dynamically add a <base>

Right, I meant it as if we need to support any pre-existing values, or a wholesale NONE would suffice.

I went with 'none' in #15580 for now.