-
-
Notifications
You must be signed in to change notification settings - Fork 7k
Avoid inline script execution for injecting CSRF token #7016
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
Conversation
9eda043
to
550a8b9
Compare
Unsubscribe |
dca1374
to
95b9ef8
Compare
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. |
95b9ef8
to
9f47c6e
Compare
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.
9f47c6e
to
91d8807
Compare
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. |
@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. |
Replying on this one seems more up to date, since #5740 seems a bit outdated (shows conflicts), #7960 does seem a bit off because |
I'm happy to add collaborators to the @encode/django-rest-framework team if asked. |
@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. |
@juspence - Invite sent. |
@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:
Then I tested again with this change, and the above error went away. |
@juspence - Great. You're welcome to approve pull requests that you're happy with. |
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.