search

wagtail / wagtail

A Django content management system focused on flexibility and user experience
https://wagtail.org
BSD 3-Clause "New" or "Revised" License
17.94k stars 3.78k forks source link

A list of widgets breaking strict Content-Security-Policy (CSP) directives #7053

Open brownaa opened 3 years ago

brownaa commented 3 years ago

Is your proposal related to a problem?

See #1288. Wagtail uses inline javascript and styles to provide useful feedback to the user. This causes conflicts with CSP directives.

This issue attempts to list all of the potential places this occurs in the Wagtail codebase.

Directives to be reviewed:

Extra Notes

script-src: 'self'

Wagtail Admin

Wagtail Admin Templates

  1. wagtail/admin/templates/wagtailadmin/

  2. wagtail/admin/templates/wagtailadmin/edit_handlers/

  3. wagtail/admin/templates/wagtailadmin/home/

  4. wagtail/admin/templates/wagtailadmin/pages/

  5. wagtail/admin/templates/wagtailadmin/permissions/includes/

  6. wagtail/admin/templates/wagtailadmin/reports/

  7. wagtail/admin/templates/wagtailadmin/shared/

  8. wagtail/admin/templates/wagtailadmin/widgets/

  9. wagtail/admin/templates/wagtailadmin/workflows/

Wagtail Contrib

Wagtail Documents

Wagtail Images

Wagtail Snippets

Wagtail Users

Wagtail Utils

style-src: 'self'

Wagtail Embeds

Wagtail Admin

Contrib

Documents

Embeds

Images

Users

gasman commented 3 years ago

Don't know if this is on your radar already, but a lot of inline <script> tags won't necessarily be in templates - in particular, form widgets inheriting from WidgetWithScript will contribute a lot of them.

brownaa commented 3 years ago

@gasman Thanks for the tip.

brownaa commented 3 years ago

script-src is done.

brownaa commented 3 years ago

style-src is done

brownaa commented 3 years ago

Example of how to add nonce to an inline <script> element. https://github.com/brownaa/wagtail/commit/0d497580581399cb52c148dd17899550f7febb94

Example of how to add nonce to an inline style element. https://github.com/brownaa/wagtail/commit/bc2738e540731fe47dcdc6273f6122a36e45eaa8

Based on my testing, it does not appear that we could add a nonce to a style attribute like <div style="display:none" nonce="r@nd0m">

thibaudcolas commented 3 years ago

Reading up / based on our discussion in Slack, we’ll need to get rid of those style="" attributes in HTML/Python, and in JavaScript switch any usage of setAttribute('style') to adding individual properties on the style DOM attribute: https://stackoverflow.com/a/29089970/1798491.

I couldn’t see any setAttribute('style') in Wagtail’s JS code, but there could be in our dependencies.

For style attributes in HTML, I could see two based on the list above that will be tricky to change:

Does some clever math. This would either require a <style> attribute with a nonce and adding styles via a unique id to each iframe… or I think we might be able to refactor this to use the more modern aspect-ratio CSS property.

This might be easier by setting the values via JS.

All the other style attributes in HTML can either be replaced with CSS classes, or the hidden HTML attribute.

rob101 commented 2 years ago

Why not solve this with a Middleware class? E.g. assuming you are using django-csp to generate a nonce, you could follow something like this with BS4 to add attributes to tags:

from __future__ import annotations
from typing import Callable
from bs4 import BeautifulSoup

from django.http import HttpRequest, HttpResponse

class ScriptMiddleware:
    def __init__(self,
                 get_response: Callable[[HttpRequest], HttpResponse]) -> None:
        self.get_response = get_response

    def __call__(self, request: HttpRequest) -> HttpResponse:
        response = self.get_response(request)
        content = response.content.decode(response.charset)

        # Add nonce to scripts
        soup = BeautifulSoup(content)
        els = soup.find_all('script')
        for el in els:
            el.attrs['nonce'] = str(request.csp_nonce)
        response.content = str(soup)

        return response
gasman commented 2 years ago

@rob101 Because that's defeating the point of having CSP in the first place. If someone can sneak a <script> tag into the template through an XSS attack, this middleware will blindly mark it as trusted even though there's no checking to see where it came from.

rob101 commented 2 years ago

@gasman XSS is usually client side isn't it, typically a browser side script injection? The middleware runs before DOM load so bad actor script tags would not be caught, unless I am missing something?

https://owasp.org/www-community/Types_of_Cross-Site_Scripting

gasman commented 2 years ago

@rob101 OK, yep, the third type described there (DOM Based XSS) is one that could conceivably be stopped by CSP with this middleware approach (in a setup where the malicious script isn't initially present in executable form as part of the served document, but is placed there as a result of running trusted client-side code with security flaws). I don't believe that's the most common type by a long way, though.

lb- commented 2 years ago

Workflow history detail will be solved via https://github.com/wagtail/wagtail/pull/8626

lb- commented 2 years ago

RFC 78 created https://github.com/wagtail/rfcs/pull/78 - with the goal of giving us a practical solution to widget scripts (amongst other things).

lb- commented 2 years ago

wagtail/contrib/redirects/templates/wagtailredirects/choose_import_file.html should be fixed by https://github.com/wagtail/wagtail/pull/8654

script was not even used for this view.

lb- commented 2 years ago

Login page - inline script to focus on username field has been replaced with autofocus attribute.

Via #8925

lb- commented 1 year ago

10045 will fix some of the script elements used as template tags in documents|images/mulitiple/add.

Hopefully will get a chance to update this main issue with all the things that are checked off.