Shasum check is not robust
First of all, it's great to see that the longstanding issue of trusting a remote SHASUMS256.txt file has been fixed. Huge thanks for that!
There is a bug in the impl though: it doesn't recheck that shasums match when it pulls a file out of cache. That happens in https://github.com/electron/get/blob/main/src/index.ts#L72-L82 -- that codepath returns the file from cache without any shasum verification, regardless of how it got into the cache.
Steps to reproduce, variant 1
-
npm i electron - replace the cache file (look in
~/.cache/electron) with something else -
npm i electronin another dir -- the tampered file will be used without any shasum check
Steps to reproduce, variant 2
-
npm i electronto get npm packages -
rm -rf ~/.cache/electronto clear cache - Replace the hash in
node_modules/electron/checksums.jsonto simulate a hash mismatch -
rm -rf ~/node_modules/electron/distto trigger reinstall -
node ./node_modules/electron/install.jsnow fails, as it should -
electron_use_remote_checksums=1 node ./node_modules/electron/install.jssucceeds (expected) -
rm -rf ~/node_modules/electron/distto trigger reinstall -
node ./node_modules/electron/install.jssucceeds, despite the hash mismatching the expected one inchecksums.json.
I.e. if the first call to whatever put the package in cache has been done e.g. with electron_use_remote_checksums env var, or without checksums option, or with unsafelyDisableChecksums option, the follow up call which has checksums option set and expects them to be validated against those supplied ones won't get them validated.
Same as when the cache has been corrupted or tampered, allowing e.g. a long-term CI poisoning via cache artifacts.