WordPress.org

Make WordPress Core

Opened 2 months ago

Last modified 3 weeks ago

#51407 assigned enhancement

Remove inline event handlers and JavaScript URIs for Strict CSP-compatibility

Reported by: enricocarraro Owned by: adamsilverstein
Milestone: Awaiting Review 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:

  1. Add nonces to <script> elements
  2. Refactor inline event handlers and javascript: URIs
  3. Refactor calls to JS APIs incompatible with CSP
  4. Serve the Content-Security-Policy header

More on CSP.

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 (6)

This ticket was mentioned in PR #551 on WordPress/wordpress-develop by enricocarraro.


2 months ago

  • Keywords has-unit-tests added

This ticket was mentioned in Slack in #core-js by enricocarraro. View the logs.


2 months ago

#3 @adamsilverstein
2 months ago

  • Owner set to adamsilverstein
  • Status changed from new to assigned

#4 @enricocarraro
2 months ago

As suggested by @adamsilverstein, here's a list of page changes:

  1. Invalid Comment ID 'Go Back' button and MultiSite 'Back' button
    • Originally in src/wp-admin/comment.php] moved to src/js/_enqueues/admin/common.js
    • commit
  2. Custom Image Header: 'Blog Name' anchor
    • Originally in src/wp-admin/includes/class-custom-image-header.php moved to src/js/_enqueues/admin/custom-header.js
    • commit
  3. Link Manager: 'Delete' button
    • Originally in src/wp-admin/includes/class-wp-links-list-table.php moved to src/js/_enqueues/admin/link-manager.js
    • commit
  4. Link details: 'Delete' button
    • Originally in src/wp-admin/includes/meta-boxes.php moved to src/js/_enqueues/admin/link.js
    • commit
  5. 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 to src/js/_enqueues/admin/media-events.js
    • commit
  6. 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 to src/js/_enqueues/admin/media-events.js
    • same as above
  7. Media Library List View: 'Delete' and 'Attach' button
    • Originally in src/wp-admin/includes/class-wp-media-list-table.php moved to src/js/_enqueues/admin/media.js
    • commit
  8. Advanced Edit form: "Get shortlink"
    • Originally in src/wp-admin/edit-form-advanced.php moved to src/js/_enqueues/admin/post.js
    • commit
  9. Setup Config: "Go back" button on config failure
    • Originally in src/wp-admin/setup-config.php moved to src/js/_enqueues/admin/setup-config.js
    • commit
  10. Theme List: Delete button
    • Originally in src/wp-admin/includes/class-wp-themes-list-table.php moved to src/js/_enqueues/admin/themes-list.js
    • commit
  11. 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 to src/js/_enqueues/lib/image-edit.js
    • commit
  12. 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 and src/js/_enqueues/lib/metabox-events.js moved to src/js/_enqueues/lib/metabox-events.js
    • commit
  13. Dashboard: reply to comment button
    • Originally in src/wp-admin/includes/dashboard.php and src/js/_enqueues/admin/edit-comments.js
    • commit
  14. Customize Menu view: "widget areas" and "Widgets panel" button
    • Originally in src/wp-includes/class-wp-customize-nav-menus.php moved to src/js/_enqueues/wp/customize/nav-menus.js
    • commit
  15. 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 to src/js/_enqueues/wp/customize/widgets.js
    • commit
  16. Theme and Plugin Editor: "Docs Lookup" button
    • Originally in src/wp-admin/plugin-editor.php and src/wp-admin/theme-editor.php
    • commit
  17. Async Upload: "Dismiss" button
    • Target src/wp-admin/async-upload.php
    • commit
  18. Theme List Install: "Try Again" Button
    • Target: src/wp-admin/includes/class-wp-theme-install-list-table.php
    • commit
  19. Template Meta Form: "Enter" new
    • Target: src/wp-admin/includes/template.php
    • commit
  20. Comment Template: "Reply" button
    • Target: src/wp-includes/comment-template.php
    • commit
  21. Default wp_die handler: "Go back" button
    • Target: src/wp-includes/functions.php
    • commit
  22. ThickBox: iframe loading
    • Target: src/js/_enqueues/vendor/thickbox/thickbox.js
    • commit

List of efforts on WordPress JavaScript dependencies:

  • TinyMCE compat3x plugin: PR
  • MediaElement volume: PR
Last edited 5 weeks ago by enricocarraro (previous) (diff)

#5 @enricocarraro
7 weeks 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 @hellofromTonya
3 weeks 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.

Note: See TracTickets for help on using tickets.