[Security] Malicious code in test/resolver/multirepo

[aashutoshrathi]

SNYK-JS-MONOREPOSYMLINKTEST-5865510

This path seems to be causing issues node_modules/resolve/test/resolver/multirepo/package.json

Is there any fix version for this? Should the package be shipping test folder with it?

[ljharb]

Because it's a private package that just coincidentally has the same name as the malicious one, it is a false positive - so whatever tool is flagging this repo is broken, and you should strongly reconsider using a tool that is this naive about npm package names.

Yes, tests should always be shipped with packages so that npm explore foo && npm install && npm test always works.

Duplicate of #303. Duplicate of #291. Duplicate of #288. Duplicate of #304. Duplicate of #305. Duplicate of #306.

[slitsevych]

The tool in question is a very well known Snyk cloud-native security scanner. And it is being used by AWS Inspector service (as well as in numerous CI/CD pipelines, by other cloud providers, etc) which, in our case, as of yesterday started to report critical vulnerability finding concerning "monorepo-symlink-test".

I'm pretty sure that will be more complaints on that matter as it does not seem to come from Snyk directly, it's rather due to the fact that NPM has "monorepo-symlink-test" listed as "Security holding package" (package contained malicious code and was removed from the registry by the npm security team) at https://www.npmjs.com/package/monorepo-symlink-test

So maybe it would make sense to rename the private package inside the "test" folder to avoid conflicts with NPM registry?

[aashutoshrathi]

Because it's a private package that just coincidentally has the same name as the malicious one, it is a false positive - so whatever tool is flagging this repo is broken, and you should strongly reconsider using a tool that is this naive about npm package names.

Yes, tests should always be shipped with packages so that npm explore foo && npm install && npm test always works.

Duplicate of #303. Duplicate of #291. Duplicate of #288. Duplicate of #304. Duplicate of #305. Duplicate of #306.

😮 ah! makes sense. Couldn't have guessed it's just the same name.
Also, thanks for clarifying that tests should be shipped.

[ljharb]

heads up @lirantal that snyk is wrongly warning on a package.json file that has private: true merely because it has the same name as a malicious package.

@slitsevych no, i refuse to do that, because tools that wrongly call this out should be notified.

[santialbo]

@ljharb, I completely acknowledge your perspective on the limitations of code scanners, and you are absolutely right about their shortcomings. However, I'd like to kindly emphasize the significant impact this issue is having on lots of engineers and development teams. Since this is being flagged as a critical vulnerability, it subsequently leads to an influx of inquiries and concerns.

Also, many of these engineers lack the authority or flexibility to switch to alternative code scanner services. As a result, they find themselves in a frustrating situation with limited options to address this issue.

There are already 10 reported issues on this matter, and I anticipate more will follow. You are able to fix it easily and make life easier for a lot of folks and for yourself.

[ljharb]

All the more reason to keep it, so the organizations that wrongly make such a scanner a blocker, as opposed to merely advisory, can correct their process.

[slitsevych]

@santialbo, I must admit that I've had to find a workaround around this until some moment in the future when this is no longer an issue... so I've ended up forking the repository, removing references to the "monorepo-symlink-test" package, publishing it and then supplying to the packages which require resolve in the following "overriding" manner (in "package.json"):

"overrides": {
    "knex": {
      ".": "^2.5.1",
      "resolve": "npm:[email protected]"
    },
    "rechoir": {
      ".": "^0.8.0",
      "resolve": "npm:[email protected]"
    }
  }

In this way all those package basically get the same code (which they does not seem to require that much) but which passes the scans. Possibly that's an overkill, but as a DevOps guy I'm just freaking tired of getting emails regarding this "critical" vulnerability

[ljharb]

lol that's certainly an approach in the meantime. but as a devops guy, i'd suggest removing a scanner that doesn't let you easily suppress specific warnings, especially since the vast majority of CVEs in the JS ecosystem are false positives.

[steveanderson2]

Because it's a private package that just coincidentally has the same name as the malicious one, it is a false positive - so whatever tool is flagging this repo is broken, and you should strongly reconsider using a tool that is this naive about npm package names.

Yes, tests should always be shipped with packages so that npm explore foo && npm install && npm test always works.

Duplicate of #303. Duplicate of #291. Duplicate of #288. Duplicate of #304. Duplicate of #305. Duplicate of #306.

Several scanners are reporting this. There are 4 engineers in my dept that have to waste time looking into it. Probably a couple dozen more across my company. Probably a couple hundred more across the user base. Your answer is: "the scanners are wrong, it's only flagging my test code because it has the same name as some malicious code. It should look inside and see that this is not that malicious package." While you may be technically right, your refusal to rename your test code to something that does not trip alarms all over this place is wasting time for a lot of people.

[ljharb]

@steveanderson2 yes, it's indeed a waste of time for lots of people.

However, even if I made this change, i'd have to fix it in both v1 and v2, and you'd all still have to update your lockfiles to include the new release. That's also a lot of work for me who, unlike all those engineers you mention, isn't getting paid for their time.

[aashutoshrathi]

I would have patched it on my end if it were just inside our own package, but it's the docker node images that are getting affected by this.

[lirantal]

@slitsevych @aashutoshrathi can you please share how and where exactly are you getting these Snyk scan vulnerability results?

I tried reproducing via these and we Snyk doesn't alert that malicious package false positive:

1. If you connect this GitHub repository to Snyk
2. If you clone the repo and scan for vulnerabilities (either via snyk test or snyk test --all-projects)
3. Install the resolve package in a new npm project and run snyk test

What sort of scenario are you doing that you get the malicious package reported?

p.s.
This is really more of a Snyk issue we should figure out than the maintainer and probably better to bring up as a

[aashutoshrathi]

Hey! snyk test doesn't throw error. As it is raised by inspector not due to our project.
But when you install yarn on node:18.17-alpine3.18. It says vulnerable code at usr/local/share/.config/yarn/global/node_modules/resolve/test/resolver/multirepo/package.json

[lirantal]

I don't understand how yarn is related to this. Can you provide reproducible steps?

[aashutoshrathi]

if you link a project to AWS, which builds on Dockerfile it looks like this.

FROM node:18.17-alpine3.18
RUN npm i -g npm
...

Also, just realized AWS Inspector has also raised an alarm on the node_modules of our project other than the global folder of yarn.
Pointing to this file -> node_modules/resolve/test/resolver/multirepo/package.json

[santialbo]

For me it's the same as @aashutoshrathi, Amazon inspector.

[lirantal]

I'll ask internally how this works integration works but it indeed seems to be scoped and limited only to this Amazon inspector integration and not the native Snyk tooling.

[ljharb]

cc @mattapperson, can you help me find the right person at Amazon to address this false positive?

[ShadabFaiz]

What if I remove this package after installing all the packages ?