lirantal / lockfile-lint

Lint an npm or yarn lockfile to analyze and detect security issues
Apache License 2.0
780 stars 35 forks source link

Are lockfiles actually a vulnerability? #108

Closed jeznag closed 11 months ago

jeznag commented 3 years ago

<Disclaimer> I am not trying to hack anyone else, just trying to figure out if this a real problem that I need to worry about for my team </Disclaimer>

I tried following the steps here: https://snyk.io/blog/why-npm-lockfiles-can-be-a-security-blindspot-for-injecting-malicious-modules/

but it doesn't actually result in a vulnerability.

I followed these steps:

  1. Created a malicious fork of the package nprogress for demonstration purposes (https://github.com/nprogres/nprogress/commit/308c38d90271ceaa106597ca97aa103cb332921f)

  2. Ran yarn add nprogres/nprogress

  3. Committed yarn.lock but did not change package.json

  4. Did a fresh clone incorporating that commit.

  5. Confirmed that yarn.lock was pointing to the git version of progress

  6. Ran yarn install

Result: yarn used the version from npm not my version

I also tried:

a. bumping the version of nprogress in package.json to 0.2.3 (npm version is 0.2.1) but this didn't work, it says Couldn't find any versions for "nprogress" that matches "^0.2.3"

b. committing package.json - this works but means that it is rather obvious because it has

"nprogress": "nprogres/nprogress" which a human will easily notice

Question:

Is it possible that yarn has patched the vulnerability and will only use git based modules if they are present in package.json? If so, this whole library does not seem relevant any more and the snyk blog post could be archived and this github repo marked as outdated.

papb commented 3 years ago

Hi, can you try yarn install --frozen-lockfile instead of yarn install in your step 6?

lirantal commented 3 years ago

@jeznag did you also make sure the cache on disk is cleared and that node_modules/ dir doesn't exit anymore? I'd also try @papb's suggestion, as it enforces sources of the lockfile, rather than the package manager figuring out "what's best"

I haven't tested recently, to say whether this behavior was changed with newer versions of yarn, but regardless, there's still benefit in ensuring a trusted set of sources you allow to fetch deps from, and not open this up for rogue sources.

jeznag commented 3 years ago

Hi, can you try yarn install --frozen-lockfile instead of yarn install in your step 6?

Yup I tried that. It makes no difference. The flag seems to just not update the lockfile

Don’t generate a yarn.lock lockfile and fail if an update is needed.

I tried clearing the cache and removing node_modules. Same result

image

Whereas if I install it directly, I get the malicious version

image

image