Make WordPress Core

Opened 9 years ago

Closed 4 years ago

#33948 closed enhancement (maybelater)

Implement subresource integrity (SRI)

Reported by: johnbillion's profile johnbillion Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: Script Loader Keywords: dev-feedback needs-testing has-patch
Focuses: Cc:

Description

Chrome 45 (released 1st September) has shipped with subresource integrity. Firefox will ship with it in 43 (expected December 2015). Browsers that support SRI will block a script or style resource from loading if the hash of its contents doesn't match the precomputed hash in the integrity attribute for the resource.

We should investigate adding the integrity attribute to core's scripts and styles. While it affords little protection by default (because a hacked site could also have its hashes recomputed), it does protect sites that offload CSS and JS to a CDN.

The hashes can be computed during the build process.

One concern I have is that this introduces a requirement to re-compute the file hash every time a CSS or JS file is changed during development, which will be a complete pain for anyone hacking on WordPress. The answer could be to exclude the integrity attribute when WP_DEBUG is set to true (and thus, only compute the hashes for minified files).

Attachments (1)

wp-scripts-sri-patch.diff (13.0 KB) - added by joe_bopper 8 years ago.

Download all attachments as: .zip

Change History (13)

#1 @jorbin
9 years ago

I could imagine having the hashes update based on the build script. It's not something we would want to manage manually.

https://www.npmjs.com/package/grunt-sri looks like it could be a start for investigating this.

#2 @dbarlett
9 years ago

What about adding an optional integrity parameter to wp_register_script(), etc for non-core CSS and JS?

#3 @johnbillion
9 years ago

@dbarlett Yep that could work. To prevent parameter overload in the wp_enqueue_*() functions it would probably have to end up being some sort of $arguments or $attributes parameter.

Another concern I thought of regarding core implement SRI on core assets: A plugin might be overriding assets with a CDN which might not return exactly the same file as core, and then SRI would block the asset from loading. This might actually be a blocker.

#4 @joe_bopper
8 years ago

  • Keywords needs-testing has-patch added; needs-patch removed

Hi,

I have created and attached a patch that allows the application (and removal) of extra attributes to both a wp_script and wp_style (that is, enqueued <script> and <link> tags). The functions for these are:

$handle = 'my-script';
$attrs = array( 'async'=>'true' );
wp_script_add_extra_attributes( $handle, $attrs );

and

wp_script_delete_extra_attributes( $handle, $attrs );

I've then added a function for the integrity specific case:

function wp_script_integrity( $handle, $hash, $crossorigin = 'anonymous' ){...}

Note: style functions just replace script with style in the function name.

I think this should suitably fulfill this ticket.

It appears to me that the main purpose of SRI is to prevent XSS when using third-party resources (e.g. cdn). Therefore, the need for a function to create hashes for local scripts seems moot (though I'm not against the idea).

Further note: I added a new formatting function esc_attr_name as well. This simply ensures no bad characters are used in attribute names and that it is not the empty string.

Cheers.

#5 @swissspidy
8 years ago

Sounds like a duplicate of #22249.

#6 @joe_bopper
8 years ago

There's certainly a large amount of overlap. I'm happy to adjust my patch to fit should the other one be adopted.

#7 @meitar
7 years ago

In case it's of interest, the WP-SRI plugin (https://github.com/meitar/wp-sri) has been doing this as a feature plugin for a while.

#9 @johnbillion
7 years ago

Interestingly, it's unlikely that anyone would have used SRI for that script due to its potentially dynamic nature. https://www.troyhunt.com/the-javascript-supply-chain-paradox-sri-csp-and-trust-in-third-party-libraries/

But that's beside the point. I'd like to get some form of SRI support into core. This is probably blocked by #22249.

#10 @Otto42
7 years ago

+1

This would also be useful for plugins that include code from other sevrices. We've recently had a case where a service had their javascript changed to include coinhive mining code, ostensibly without their knowledge. If wp_enqueue_script included the ability for their plugin to define the integrity hash of that external JS, then the code would have been blocked.

As it is, they can certainly change the plugin to output their own script tag to include such a hash, but having this built into the scripts/styles system would be very helpful.

#11 @zakkath
6 years ago

Any movement on this?

#12 @johnbillion
4 years ago

  • Milestone Awaiting Review deleted
  • Resolution set to maybelater
  • Status changed from new to closed
Note: See TracTickets for help on using tickets.