Opened 4 years ago
Last modified 12 months ago
#51407 assigned enhancement
Remove inline event handlers and JavaScript URIs for Strict CSP-compatibility
Reported by: | enricocarraro | Owned by: | adamsilverstein |
---|---|---|---|
Milestone: | Future Release | Priority: | normal |
Severity: | normal | Version: | 4.8 |
Component: | Security | Keywords: | has-patch 2nd-opinion has-unit-tests |
Focuses: | javascript | Cc: |
Description
Content Security Policy is a mechanism designed to make applications more secure against common web vulnerabilities, particularly cross-site scripting. It is enabled by setting the Content-Security-Policy HTTP response header.
An application can add a critical defense-in-depth layer against markup injection attacks by adopting a strict policy that prevents the loading of untrusted scripts or plugins.
A basic policy (nonce + strict-dynamic + unsafe-eval) would block more than 40% of the XSS sinks.
To make an application compatible with strict CSP, it is necessary to make changes to HTML templates and client-side code and add the policy header:
- Add nonces to <script> elements
- Refactor inline event handlers and javascript: URIs
- Refactor calls to JS APIs incompatible with CSP
- Serve the Content-Security-Policy header
Since these PRs are quite significant, I separated the one regarding script nonces (#39941) and the one on inline event handlers and JavaScript URIs to facilitate code reviews.
This patch builds on top of this one (use this link to compare the two branches easily); I introduced some new JavaScript files where I moved inline event handlers and JavaScript URIs to, in particular:
- src/js/_enqueues/admin/link-manager.js: handles link deletion confirmation;
- src/js/_enqueues/admin/media-events.js: events related to the media library and media details;
- src/js/_enqueues/admin/setup-config.js: event handler for try again button;
- src/js/_enqueues/admin/themes-list.js: theme deletion confirm event handler;
- src/js/_enqueues/lib/metabox-events.js: event handlers for various meta boxes.
I also moved various event handlers to already existing JavaScript files.
In some cases, I couldn't move the inline event handlers to a file because 'user-defined' ajax calls could load the generated HTML, so I moved them to an inline script.
Change History (17)
This ticket was mentioned in PR #551 on WordPress/wordpress-develop by enricocarraro.
4 years ago
#1
- Keywords has-unit-tests added
This ticket was mentioned in Slack in #core-js by enricocarraro. View the logs.
4 years ago
#4
@
4 years ago
As suggested by @adamsilverstein, here's a list of page changes:
- Invalid Comment ID 'Go Back' button and MultiSite 'Back' button
- Originally in
src/wp-admin/comment.php
] moved tosrc/js/_enqueues/admin/common.js
- commit
- Originally in
- Custom Image Header: 'Blog Name' anchor
- Originally in
src/wp-admin/includes/class-custom-image-header.php
moved tosrc/js/_enqueues/admin/custom-header.js
- commit
- Originally in
- Link Manager: 'Delete' button
- Originally in
src/wp-admin/includes/class-wp-links-list-table.php
moved tosrc/js/_enqueues/admin/link-manager.js
- commit
- Originally in
- Link details: 'Delete' button
- Originally in
src/wp-admin/includes/meta-boxes.php
moved tosrc/js/_enqueues/admin/link.js
- commit
- Originally in
- Media : 'Delete' button, 'Edit Image' button, 'Cancel' delete button, Use Featured Image, 'Cancel' button, 'Insert gallery', 'Update gallery settings'
- Originally in
src/wp-admin/includes/media.php
moved tosrc/js/_enqueues/admin/media-events.js
- commit
- Originally in
- Media Insert URL Form: src 4 radio inputs for alignment, "Go", "Link to image" and "None" buttons
- Originally in
src/wp-admin/includes/media.php
moved tosrc/js/_enqueues/admin/media-events.js
- same as above
- Originally in
- Media Library List View: 'Delete' and 'Attach' button
- Originally in
src/wp-admin/includes/class-wp-media-list-table.php
moved tosrc/js/_enqueues/admin/media.js
- commit
- Originally in
- Advanced Edit form: "Get shortlink"
- Originally in
src/wp-admin/edit-form-advanced.php
moved tosrc/js/_enqueues/admin/post.js
- commit
- Originally in
- Setup Config: "Go back" button on config failure
- Originally in
src/wp-admin/setup-config.php
moved tosrc/js/_enqueues/admin/setup-config.js
- commit
- Originally in
- Theme List: Delete button
- Originally in
src/wp-admin/includes/class-wp-themes-list-table.php
moved tosrc/js/_enqueues/admin/themes-list.js
- commit
- Originally in
- Image Edit: help toggles, text fields for crop and scale and buttons for Preview, Crop, Rotate, Flip, Undo, Redo, Cancel, Submit and Scale
- Originally in
src/wp-admin/includes/image-edit.php
moved tosrc/js/_enqueues/lib/image-edit.js
- commit
- Originally in
- Block Editor: metabox location form; Media Details : "Delete" button, "Add new comment" button, "Reply" button, "Show comments" button
- Originally in
src/wp-admin/includes/post.php
andsrc/js/_enqueues/lib/metabox-events.js
moved tosrc/js/_enqueues/lib/metabox-events.js
- commit
- Originally in
- Dashboard: reply to comment button
- Originally in
src/wp-admin/includes/dashboard.php
andsrc/js/_enqueues/admin/edit-comments.js
- commit
- Originally in
- Customize Menu view: "widget areas" and "Widgets panel" button
- Originally in
src/wp-includes/class-wp-customize-nav-menus.php
moved tosrc/js/_enqueues/wp/customize/nav-menus.js
- commit
- Originally in
- Customize Widgets: Nav Menu widget "Create some" button when no menus exist
- Originally in
src/wp-includes/widgets/class-wp-nav-menu-widget.php
moved tosrc/js/_enqueues/wp/customize/widgets.js
- commit
- Originally in
- Theme and Plugin Editor: "Docs Lookup" button
- Originally in
src/wp-admin/plugin-editor.php
andsrc/wp-admin/theme-editor.php
- commit
- Originally in
- Async Upload: "Dismiss" button
- Target
src/wp-admin/async-upload.php
- commit
- Target
- Theme List Install: "Try Again" Button
- Target:
src/wp-admin/includes/class-wp-theme-install-list-table.php
- commit
- Target:
- Template Meta Form: "Enter" new
- Target:
src/wp-admin/includes/template.php
- commit
- Target:
- Comment Template: "Reply" button
- Target:
src/wp-includes/comment-template.php
- commit
- Target:
- Default wp_die handler: "Go back" button
- Target:
src/wp-includes/functions.php
- commit
- Target:
- ThickBox: iframe loading
- Target:
src/js/_enqueues/vendor/thickbox/thickbox.js
- commit
- Target:
List of efforts on WordPress JavaScript dependencies:
#5
@
4 years ago
@whyisjake Do you know anybody that could help in the review process? I would appreciate some more eyes on it since there isn't much time until the 5.6 beta phase.
I would also like to stress how important strict CSP would be in protecting WordPress users.
#6
@
4 years ago
- Version changed from trunk to 4.8
This ticket does not seem to be directly caused by Version 5.6 (ie trunk
). It does reference being built upon #39941, which has a Version of 4.8. Changing the version to match that ticket.
However, if the version is different, please advise or update.
#8
in reply to:
↑ description
@
4 years ago
I think a better way would be not to use nonces (too complex to configure on the web server side etc.). I recommend to make all Javascript strictly as external files. Then, I dont need to configure the nounces in the headers.
Othrwise I strongly support strict CSP. The new block editor is a security disaster:
unsafe-inline, unsafe-eval, external references to google fonts...
A good out of the box WordPress installation must work with the following CSP for all areas (especially the admin area):
Content-Security-Policy: default-src 'none'; base-uri 'self'; script-src 'self'; style-src 'self'; img-src 'self'; connect-src 'self'; font-src 'self'; object-src 'none'; media-src 'none'; child-src 'self'; form-action 'self'; frame-ancestors 'none'; navigate-to 'self'; block-all-mixed-content
This ticket was mentioned in Slack in #core by hellofromtonya. View the logs.
4 years ago
#10
@
4 years ago
Notes from last scrub:
@whyisjake raised his hand to review for 2nd opinion.
@adamsilverstein thought there's still time for this ticket to land in 5.7 Beta 1.
#11
@
4 years ago
- Milestone changed from 5.7 to 5.8
5.7 Beta 1 is happening shortly. Ran out of time to get this ticket into the release. Punting to 5.8.
If any maintainer or committer feels this can be resolved in time, or wishes to assume ownership during a specific cycle, feel free to update the milestone accordingly.
This ticket was mentioned in Slack in #core by chaion07. View the logs.
4 years ago
#14
@
3 years ago
- Milestone changed from 5.9 to Future Release
Marking as future release for now and will look for opportunities to circle back to this work.
#15
@
3 years ago
I agree and strongly support that all Javascript as well as CSS should be strictly external for higher security with CSP.
#16
@
19 months ago
agree with @jornfranke not making it lead bad dev/sys admin to do ugly stuff like remove the inline source code.
Any news about that achievment ?
Any things about good practices in dev advices (core, plugins)
#17
@
12 months ago
Hi,
Gutenberg did some fix related to CSP:
https://github.com/WordPress/gutenberg/issues/42513
Do you have an idea when this could be included?
Thank you.
Best regards
Trac ticket: https://core.trac.wordpress.org/ticket/51407