Tags are injected with integrity="undefined" on v1.5.0

[jahed]

I'm on Webpack 4.44.2 and noticed after upgrading to this plugin's v1.5.0 release that injected tags have an undefined integrity. Tags inserted via html-webpack-plugin have the correct integrity, but dynamically injected tags use undefined and the integrity check is bypassed.

Here's what Firefox prints:

The script element has a malformed hash in its integrity attribute: "undefined". The correct format is "<hash algorithm>-<hash value>".

There are no errors or warnings printed during Webpack's build.

Downgrading to 1.4.1 fixes this issue so I'm assuming it's related to the changes that added Webpack 5 support in 1.5.0.

[LP1994]

It's also being discussed here.

#126

[jscheid]

@jahed thanks for reporting this. A fix for this issue is published in v1.5.1 and a security advisory created.

[jscheid]

This one shouldn't have slipped through 💩

It's a Sunday night where I am but I'll expand a bit more about the contributing factors here soon.

[LP1994]

v1.5.1!!!It's still wrong.

[jahed]

@LP1994 I haven't checked 1.5.1 myself yet but what you're experiencing is probably a browser caching bug as it's computing a SHA256 when it should be comparing a SHA512. You might need to cache bump your files (e.g. add a number to the end of the output files like CT_JS_0c8cb6.js becomes CT_JS_0c8cb6_1.js). I had a similar issue a few weeks back and a cache bump solved it.

If it's not that it may be this: https://bugs.chromium.org/p/chromium/issues/detail?id=881365

[LP1994]

@LP1994 I haven't checked 1.5.1 myself yet but what you're experiencing is probably a browser caching bug as it's computing a SHA256 when it should be comparing a SHA512. You might need to cache bump your files (e.g. add a number to the end of the output files like CT_JS_0c8cb6.js becomes CT_JS_0c8cb6_1.js). I had a similar issue a few weeks back and a cache bump solved it.

If it's not that it may be this: https://bugs.chromium.org/p/chromium/issues/detail?id=881365

Moreover, the error Sri value is different from the Sri value on the page!!! And I have cleared the cache, there should be no cache.

[LP1994]

@LP1994 I haven't checked 1.5.1 myself yet but what you're experiencing is probably a browser caching bug as it's computing a SHA256 when it should be comparing a SHA512. You might need to cache bump your files (e.g. add a number to the end of the output files like CT_JS_0c8cb6.js becomes CT_JS_0c8cb6_1.js). I had a similar issue a few weeks back and a cache bump solved it.

If it's not that it may be this: https://bugs.chromium.org/p/chromium/issues/detail?id=881365

However, 1.5.0 is still OK, but the Sri of dynamically imported files cannot be generated correctly.

Now, however, none of the Sri values work.

[jahed]

@LP1994 The value in the console log is the SHA256 hash, while yours is a SHA512, so it will be different. Either Chrome is using a cached integrity (which a cache bump can fix) or it's reporting the wrong integrity (see the Chromium issue I linked).

The integrity cache is not stored in regular browser caches so it can't be cleared through the interface. You'll have to either change the script URL (cache bump), nuke your browser profile, or find the specific cache file and delete it on the file system. The easiest is to change the script URL.

I have bumped my build to 1.5.1 and it's all working correctly on my end. 👍🏽

[LP1994]

@LP1994 The value in the console log is the SHA256 hash, while yours is a SHA512, so it will be different. Either Chrome is using a cached integrity (which a cache bump can fix) or it's reporting the wrong integrity (see the Chromium issue I linked).

The integrity cache is not stored in regular browser caches so it can't be cleared through the interface. You'll have to either change the script URL (cache bump), nuke your browser profile, or find the specific cache file and delete it on the file system. The easiest is to change the script URL.

I have bumped my build to 1.5.1 and it's all working correctly on my end. 👍🏽

thank you!!!I'll try again.

[jscheid]

Ok, I've spent some time reflecting on what went wrong here, and the bottom line is:

1. Despite an extensive battery of tests, one important test for this particular case was missing.
2. Considering the scope of the changes for 1.5.0, it would have been wise to publish a release candidate first, to allow the community to uncover potential issues, as we've done in the past.
3. We've also come to rely too much on the automated test suite and didn't do enough smoke testing.

I'm planning to take the following steps to reduce the likelihood of something like this happening again:

1. While we should now have tests to ensure (correct) integrity values are added in all cases, these tests are somewhat brittle (eg. the test added for this issue might fail for a reason other than integrity mismatch and would then trigger a false positive.) I'm going to add additional, more robust, test cases for the core functionality over the coming days.
2. The code for loading dynamic chunks at runtime should raise an error when a matching integrity value can't be found, instead of relying on browser behaviour or the test suite catching all issues. There is a small chance that this might break existing code, but really SRI is pointless unless all code loaded dynamically is protected, and so this seems a good change. This will effectively be an additional fail-safe for this particular issue.
3. We're going to establish a release protocol that explicitly includes smoke testing on a (small) range of browsers.
4. Future releases with extensive changes will first see release candidates again.

In addition, time permitting, I'm thinking of starting work on a version 2.0.0 that would be a rewrite in Typescript, and drop compatibility with Webpack < 5 (and probably also html-webpack-plugin < 5) which would further help to improve code quality by reducing footprint.

We're also going to establish a security policy, including a communications channel that can be used for reporting security issues privately.

[LP1994]

If you need a test, I'm ready to help you.

[jantimon]

@jscheid that sounds awesome :)

I talked to the webpack core team and they told me that they will probably provide us with additional stages so that we can built the html at time where the css & js files have been optimized and you can generate the correct hashes..

I'll keep you posted :)

[jpmckearin]

Sorry to resurrect this, but some clarification is needed. Does this effect only v1.5.0, or all versions before v1.5.1.
The advisory that was published by the repo states 1.5.0, but the advisory that GitHub publishes (and dependabot uses) states <1.5.1

[jahed]

@jpmckearin 1.5.0 only. It was a bug introduced in 1.5.0 and fixed in 1.5.1.