search

visma-prodsec / confused

Tool to check for dependency confusion vulnerabilities in multiple package management systems
MIT License
701 stars 93 forks source link

json: cannot unmarshal bool into Go struct field error #1

Closed visma-henriklarsen closed 3 years ago

visma-henriklarsen commented 3 years ago

Hi there

Thanks for a great tool!

We have checked a large number of package.json files. Sometimes the tool fails with this message: Encountered an error while trying to read packages from file: json: cannot unmarshal bool into Go struct field PackageJSON.bundleDependencies of type []string

For instance when running against this file npm\node_modules\decamelize\package.json

I don't know if it's a general bug or something at our end.

Thanks!

n0ncetonic commented 3 years ago

I've submitted a pull request which addresses this issue. Feel free to use that code if your time is limited and need to quickly scan packages while the pull request is reviewed

visma-henriklarsen commented 3 years ago

Thanks for your effort!

I've tried to give it a go. It looks like I get the same errors as before: Encountered an error while trying to read packages from file: json: cannot unmarshal bool into Go struct field PackageJSON.bundleDependencies of type []string

I am new to go language, maybe I am missing something. Anyway this is what I did: 1/ downloaded and installed go for windows from here: https://golang.org/doc/install 2/ git clone https://github.com/visma-prodsec/confused.git 3/ gh pr checkout 5 4/ git diff HEAD..main (to be sure I have the expected changes locally before building) 5/ navigates to the root of the source directory and tries to build using "go build ." (a new confused.exe is built) 6/ using the newly built confused.exe I scan a list of package.json files. Many succeed. But there are also a lot failing with the above mentioned error 7/ I diff 2 package.json files (1 that I can scan successfully (attached contents of mime.package.json.zip) and 1 that throws the error (attached decamelize.package.json.zip)).

mime.package.json.zip decamelize.package.json.zip

n0ncetonic commented 3 years ago

I'll take a look at this in a few hours, the npm parser is not the most robust and could definitely use some work so I may as well knock it all out at once. Thanks for providing example packages to test on.

It's pretty strange that decamelize fails though as I was explicitly testing my pull request against the package.json file in the decamelize git repo.

joohoi commented 3 years ago

I'm looking at this as we speak. The decamelize package.json in this version seems to have the following value:

  "bundleDependencies": false,

while the npm package.json documentation specifically defines the bundledDepencies to be an array of strings.

n0ncetonic commented 3 years ago

I'm looking at this as we speak. The decamelize package.json in this version seems to have the following value:


  "bundleDependencies": false,

while the npm package.json documentation specifically defines the bundledDepencies to be an array of strings.

I figured this might be the case. Main question now is whether this is common or if decamelize just breaks spec for no reason. If this is in fact a common occurrence then we can do some type checking before blindly adding to the PackageJSON struct

n0ncetonic commented 3 years ago

@visma-henriklarsen could you provide us with other public packages which are also throwing this error? Also any information you may have about non-npm package managers that may have been used to generate packages.json files which appear to be out of spec

visma-henriklarsen commented 3 years ago

@n0ncetonic and @joohoi I'm not a npm specialist, so I could be missing something trivial (I am more like devops). I lot of the packages.json files comes from our internal git repositories. It looks like some developers commited the entire node_modules tree at some point in our case. So it could be I am scanning older packages/versions of the packages, potentially superceed with updated ones that conform to the npm spec in the public repository. It's hard to say. I guess it is quite common scenario to not run on latest packages. In the attached zip file is a folder structure containing a subset of the packages.json files we are looking at. Also a powershell script used to scan the structure, plus a log file stating the output from confused.exe for each file.

package.json-files.zip

joohoi commented 3 years ago

Thanks! I think the most important takeaway from this is (regardless of the package versions your dependency tree has) that npm packages can have inconsistencies in terms of file format in their respective package.json files.

In the context of this tool, the inconsistencies should not break & exit the tool, but work with whatever valid entries there might be, as it seems to be true for the npm itself as well.