Avoid inline script execution for injecting CSRF token by lukaw3d · Pull Request #7016 · encode/django-rest-framework (original) (raw)

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 andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation11 Commits1 Checks0 Files changed

Conversation

This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters

[ Show hidden characters]({{ revealButtonHref }})

lukaw3d

Description

Scripts with type="application/json" or "text/plain" are not executed, so we can use them to inject dynamic CSRF data, without allowing inline-script execution in Content-Security-Policy.

This helps towards fixing #6069 a bit.

@Farisiimoet3

@stale

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@lukaw3d lukaw3d changed the titleSeparate CSRF data from executed javascript code to support CSP Avoid inline script execution for injecting CSRF token

Jun 25, 2022

@lukaw3d

Scripts with type="application/json" or "text/plain" are not executed, so we can use them to inject dynamic CSRF data, without allowing inline-script execution in Content-Security-Policy.

@stale

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@juspence

@tomchristie Is there any way I can help get this PR (as well as #5740 and #7960) merged? I'm happy to help test and provide feedback. Merging any of these would help to resolve #6069, which is blocking me from using stricter Cross-Site Scripting (XSS) protections.

@twbagustin

@tomchristie

@tomchristie Is there any way I can help get this PR (as well as #5740 and #7960) merged?

I'm happy to add collaborators to the @encode/django-rest-framework team if asked.
That'll give folks review and merge permissions on pull requests.

@juspence

@tomchristie I can't commit to spending any effort beyond this one feature. If that's still OK, please add me to the encode/drf team, and I'll review, test, and merge these three PRs (#5740, #7016, and #7960). Thank you!

@auvipy

@tomchristie I can't commit to spending any effort beyond this one feature. If that's still OK, please add me to the encode/drf team, and I'll review, test, and merge these three PRs (#5740, #7016, and #7960). Thank you!

you can handle this one and I can help you review and merge this. we now have 3 more new maintainers. can you test and share your feedback on this PR? after your confirmation I can merge it. the other open PR's need more works before merge.

@tomchristie

@juspence

@auvipy This looks good to me. I tested Django-REST-Framework without this change in Firefox 102.4.0esr and Chrome 106.0.5249.119, using a Content-Security-Policy like "script-src: 'self'" that doesn't allow inline scripts.

I saw the below error for the CSRF script in the developer console, like I expected:

Content Security Policy: The page’s settings blocked the loading of a resource at inline (“script-src”). 
[components:334:1](http://localhost:8008/api/v1/components)
Uncaught TypeError: window.drf is undefined
    <anonymous> http://localhost:8008/static/rest_framework/js/csrf.js:41
[csrf.js:41:17](http://localhost:8008/static/rest_framework/js/csrf.js)
    <anonymous> http://localhost:8008/static/rest_framework/js/csrf.js:41

Then I tested again with this change, and the above error went away.

@tomchristie

@juspence - Great. You're welcome to approve pull requests that you're happy with.

juspence