Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Input collision attack, managed to construct different projects that have an identical integrity checksum #260

Open
Noeda opened this issue Feb 1, 2025 · 5 comments

Comments

@Noeda
Copy link

Noeda commented Feb 1, 2025

Hey, I happened to stumble on this project while reading https://github.com/vscode-icons/vscode-icons and looked inside how the integrity hashes are computed.

Am I right in reading the code that the hash for .integrity.json is calculated by a simple concatenation of hash(filename0 + contents(filename0) + filename1 + contents(filename1) + etc.)?

I had this as proof-of-concept setup to get project1 and project2 hash to the same thing, despite having different file names (I can put this in a git repo if you want to reproduce exactly):

project1
project1/.integrity.json
project1/package-lock.json
project1/package.json
project1/lib
project1/lib/index.js
project1/lib/main.js
project2
project2/.integrity.json
project2/package-lock.json
project2/package.json
project2/libin
project2/libin/main.js
project2/libin/dex.js

(I looked at the code, made a guess in what order the files are concatenated, copied project1 to project2, then renamed lib to libin and index.js to dex.js and that resulted in the same exact hash when using nsri create -s .):

{"version":"1","hashes":{".":"sha512-5a2mjNgYlBVpAIWzt+rfneIVt7WpzErAUnwbQ5RovbNIyV2I8XJ7uZXe/jrDV3nMadU6wLexitmuXoIfdYuaLw=="}}

It seems in the code that the way the hash is constructed has nothing about feeding it file sizes, types (is it file/directory?) etc. which makes it possible to be pretty flexible about your directory structure, number of files, maybe other creative things I didn't think of on the spot, as long as you carefully set your file contents and names so that the final hashable string collides with a legitimate version of a project.

I noticed there was a verbose command line option (-v) that makes individual file integrity hashes, but it still seems to be: hash(filename + contents(filename)) which means hello.js with Hello! inside hashes to same as hell with o.jsHello! as contents.

Should maybe do an update to how the project contents are fed into the hash function? Doing a simple concatenation creates problems and I think the promises in the README.md about filenames/contents are not being fulfilled.

@Noeda
Copy link
Author

Noeda commented Feb 1, 2025

Hey @robertohuertasm sorry for the ping if this turns out to be a nothingburger, I pinged you because you seemed like an active contributor of the vscode-icons/vscode-icons (the only relatively popular project I found that uses nsri) and you might have a better sense is there anything actually concerning here.

See above but tl;dr; NSRI computes checksums for a project to use as an integrity check, but you can fool NSRI easily to give the same integrity checksum for two separate set of files. My example in my first comment is very simple so it's easy to demo, but there's much more extensive tampering you could do.

I myself don't use vscode-icons at all and don't know anything about it.

Do you have an idea how vscode-icons uses this nsri-provided integrity feature and in what capacity? (Or maybe have an idea to triage this to someone who can answer). This nsri repository hasn't had updates in a while and not sure issues are being monitored here, and I don't want to be pushy to a volunteer maintainer who maybe doesn't have the time and energy to go back to an old project.

I don't know the threat model exactly what kind of attack NSRI is meant to protect against, it wasn't spelled out in the README.md on the icons project side. I can tell it is meant to check against tampering of project files, but because I only found this project randomly from vscode-icons, which I also found randomly trying to find how to modify VSCode CSS with an extension, I have very little info how this is actually used and is it in any actually critical path.

I looked in the vscode-icons package.json and I see the 7.0.0 version, but the hashing code seems identical in nsri (i.e. still affected).

robertohuertasm added a commit to vscode-icons/vscode-icons that referenced this issue Feb 2, 2025
The integrity check was not really needed for the extension to work. It's just noise and a [maintainability burden](JimiC/nsri#260 (comment)).
@robertohuertasm
Copy link

robertohuertasm commented Feb 2, 2025

Hi @Noeda, this project was created by @JimiC in order to provide some sort of general integrity check utility that could be used in several sorts of projects.

Regarding its usage in the extension, I guess the main idea was to replicate the behavior VS Code has to detect if a extension is modifying some of its source code.

To be honest, I don't think this is even a concern for the extension anymore, and we will remove this functionality from the extension in the next release.

Hope this helps to clarify some of the doubts.

robertohuertasm added a commit to vscode-icons/vscode-icons that referenced this issue Feb 2, 2025
The integrity check is not really needed for the extension to work, and it may become a [maintainability burden](JimiC/nsri#260 (comment)). From that point of view, the idea is to simplify the extension and get rid of unnecessary dependencies.
@Noeda
Copy link
Author

Noeda commented Feb 2, 2025

Makes a lot of sense, thanks @robertohuertasm for quick turnaround! I had in my gut feeling that this project likely isn't too relevant today, but it's good to get some confirmation.

I'll leave the issue open so it's visible here on GitHub for anyone looking at NSRI as there does seem to be some out there besides the icons project, and so that I can maybe contribute with advice if @JimiC appears and wants to do anything with this.

I don't myself use either this project or the icons project, but it felt responsible to make sure it's on the radar.

@JimiC
Copy link
Owner

JimiC commented Feb 2, 2025

Hi @Noeda. Thanks for your input.

@robertohuertasm has it right.

...provide some sort of general integrity check utility
...
...replicate the behavior VS Code has to detect if a extension is modifying some of its source code.

NSRI is a node.js specific integrity tool and I explain why it works below.

I decided to make NSRI open source just in case anyone else finds the project interesting or wants to build upon.
At this point NSRI is only maintaned with dependencies updates, whenever I have the time, mood or neccesity to do so.

What NSRI tries to solve

NSRI was created to make sure that no filename, file content or project file system structure gets tampered.
There was no intention from my part to do file size checks or anything else beyond the above scope.

How NSRI ensures integrity

Although your demonstration code indeed shows how NSRI can be fooled to produce the same integrity hash, there are some key aspects here.

Let's examine some use cases:

Suppose we have a project with an integrity hash.

  • User changes the filename
    NSRI will not produce the same integrity hash

  • User changes the content of a file
    NSRI will not produce the same integrity hash

  • User changes the file system structure of the project
    NSRI will not produce the same integrity hash (we demonstrated that when we introduced NSRI's integrity tool into vscode-icons, when we had this bug report)

  • User changes the filename and content of a file to produce same integrity hash
    Although this case will not produce any warning, it basically breaks everything.

    • If user changes the icons' filename and the icons' content, then the icon will not be the same as the one shipped. The icon will not be loaded by the extension (it won't match the filename and path set in the icon's manifest), indicating tampering. This isn't a major concern for vscode-icons though but it helps on issue solving.
    • If user changes the source code filename and content, then the node.js importer will not be able to find and import the module (as it relies on correct path and file naming), breaking the extension. This also indicates tampering.

I can't think of any other use case that a user can tamper with the project and still have it working without displaying the integrity warning.
If anyone can, I would be happy to examine that use case and improve NSRI upon it.

robertohuertasm added a commit to vscode-icons/vscode-icons that referenced this issue Feb 2, 2025
The integrity check is not really needed for the extension to work, and it may become a [maintainability burden](JimiC/nsri#260 (comment)). From that point of view, the idea is to simplify the extension and get rid of unnecessary dependencies.
@Noeda
Copy link
Author

Noeda commented Feb 3, 2025

Hey @JimiC I apologize you had to see an "Input collision attack" on one of your old projects out of sudden. I wasn't sure if you were still working on this project or not, and I'm kinda hoping you have other more exciting projects going on.

For this issue: IMO it is very reasonable to not do any action at all, and just leave the project here with nothing done, no notice, nothing. This is because you are an unpaid volunteer and have provided this package for a long time, presumably thanklessly, and you owe nothing to anyone, not even for security problems. (I have done this for my most popular project, although my project wasn't security-related).

At the very least, if you decide to want to do anything at all, I'd put a notice on the README.md that there is a serious flaw in the way NSRI calculates integrity hashes. Either that or some notice that the project is not meant to provide security, and should not be relied to stop an intentional attacker, but is more about checking is your download is successful or whatnot. Just don't market this as a security project, unless the integrity hash flaw is addressed. The icons project had NSRI under "Security" title, and I think that was a misrepresentation, and is gone now.

If you want to still maintain this, aside from addressing the hashing flaw, I recommend writing to README about how would one use NSRI exactly. Like from threat model perspective. Things like: what does it protect me from, that npm or VSCode doesn't already? In what circumstances it is useful to me? I get that NSRI checks if your files are tampered. But taking myself, 99.99% time it is me, myself, who tampered with them (e.g. in VSCode I edit CSS code inside the app). What would be a real life attack that is stopped by NSRI, that is not stopped by anything else?

(I don't mean to imply the project is pointless, I simply don't know much about npm or VSCode how much they really do their checking. I would have really been interested though if the README in this project had given examples!).

Maybe as an inspiration: a real example from gaming world I've seen: There is a mod in one video game I play, where the author is vindictive against people who created competing mods, and coded his mod to tamper with those other mods if it detects they are installed. I have no idea if Node/VSCode ecosystem has ever had this kind of pettiness. That I could see NSRI-like software provides value. (but then addressing this hashing flaw would be important, or in this threat scenario a petty author would work around it!)

I can't think of any other use case that a user can tamper with the project and still have it working without displaying the integrity warning.

I'll give another hypothetical example, unlike the one in my first post I didn't test this, but this should work: Imagine the project has two files somewhere, allowlist.csv and blocklist.csv and maybe they are about allowing or blocking domains for some purpose:

allowlist.csv:
good-domain.yep.com
nice-page.flowers
blocklist.csv
malware-and-viruses.evil
horrible-website.bad
more-evil-sites.pure-evil

I could construct, as an attacker, a project that will result in the same integrity sum, where I smuggle malware-and-viruses.evil into allowlist.csv:

allowlist.csv:
good-domain.yep.com
nice-page.flowers
blocklist.csv                        <-- this is now *inside* allowlist.csv
malware-and-viruses.evil
horrible-website.bad:         <-- this is now an actual filename
more-evil-sites.pure-evil

Depending on what the allowlist/blocklist are used for, maybe that's real bad. Or a nothing. It'll depend on the project. If I take my video game real life scenario I've seen, it might be enough to just break the project and nothing else, to cause nuisance.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants