-
Notifications
You must be signed in to change notification settings - Fork 0
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
Comments
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 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). |
The integrity check was not really needed for the extension to work. It's just noise and a [maintainability burden](JimiC/nsri#260 (comment)).
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. |
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.
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. |
Hi @Noeda. Thanks for your input. @robertohuertasm has it right.
NSRI is a node.js I decided to make NSRI open source just in case anyone else finds the project interesting or wants to build upon. What NSRI tries to solveNSRI was created to make sure that no filename, file content or project file system structure gets tampered. How NSRI ensures integrityAlthough your demonstration code indeed shows how NSRI can be Let's examine some use cases: Suppose we have a project with an integrity hash.
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. |
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.
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'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:
I could construct, as an attacker, a project that will result in the same integrity sum, where I smuggle
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. |
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 ofhash(filename0 + contents(filename0) + filename1 + contents(filename1) + etc.)
?I had this as proof-of-concept setup to get
project1
andproject2
hash to the same thing, despite having different file names (I can put this in a git repo if you want to reproduce exactly):(I looked at the code, made a guess in what order the files are concatenated, copied project1 to project2, then renamed
lib
tolibin
andindex.js
todex.js
and that resulted in the same exact hash when usingnsri create -s .
):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 meanshello.js
withHello!
inside hashes to same ashell
witho.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.
The text was updated successfully, but these errors were encountered: