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

npm audit gives malware warning? #12

Closed
koesper opened this issue Feb 22, 2023 · 15 comments
Closed

npm audit gives malware warning? #12

koesper opened this issue Feb 22, 2023 · 15 comments

Comments

@koesper
Copy link

koesper commented Feb 22, 2023

When installing @piwikpro/ngx-piwik-pro, npm audit throws a critical warning.
(i needed to install it with --force, because of issue #9: angular 14/15 support)

piwik-pro-angular-tracking  *
Severity: critical
Malware in piwik-pro-angular-tracking - https://github.com/advisories/GHSA-93jq-pwrf-g6h6
No fix available
node_modules/piwik-pro-angular-tracking

The report is for all versions, not just the latest, or a specific fork
(Apologies to @andrii-lundiak, because i first thought the problem was in his PR #11, but it appears to be in all versions)

I looked through the code, and havent found anything suspicious myself, but this is very worrying

@andrii-lundiak
Copy link

No worries @koesper I got ur first comment right. Anyhow thank you for ANY feedback here, coz I believe it will increase a chance to get attention to the problem - on any levels this package has.

@andrii-lundiak
Copy link

andrii-lundiak commented Feb 22, 2023

@koesper what Node/npm version you have where u try to install this package from its latest release?

As you may have seen in my PR, I recreated lock file also within NodeJS 16 to support NEW API for lock files. So I assume if you have Node 10 or 12 OR 18 or 19 you may receive different output in regards to audit results.

But in general I understand your doubt and concern in regards to GHSA-93jq-pwrf-g6h6.

@andrii-lundiak
Copy link

andrii-lundiak commented Feb 22, 2023

@koesper look I have simple npm project with express and webpack and then I just install this package and it does NOT warn my about any vulnerabilities

image

@koesper
Copy link
Author

koesper commented Feb 22, 2023

@andrii-lundiak I noticed that, and had the same result on my fork, where i basically copied the changes in your pullrequest.
I'm running the LTS versions:

npm --version
9.5.0
node --version
v18.14.2

@andrii-lundiak
Copy link

andrii-lundiak commented Feb 22, 2023

And btw @koesper if you DIRECTLY install piwik-pro-angular-tracking then yes it's kinda suspicions component:

its node_modules/piwik-pro-angular-tracking/readme (if I install locally to my project):

# Security holding package

This package contained malicious code and was removed from the registry by the npm security team. A placeholder was published to ensure users are not affected in the future.

Please refer to www.npmjs.com/advisories?search=piwik-pro-angular-tracking for more information.

its node_modules/piwik-pro-angular-tracking/package.json:

{
  "name": "piwik-pro-angular-tracking",
  "version": "0.0.1-security",
  "description": "security holding package",
  "repository": "npm/security-holder"
}

@andrii-lundiak
Copy link

andrii-lundiak commented Feb 22, 2023

Yes, @piwikpro/ngx-piwik-pro is NOT the same package per npmjs registry terms as piwik-pro-angular-tracking.

but YES @piwikpro/ngx-piwik-pro package.json DOES HAVE wrong name:
image

@koesper
Copy link
Author

koesper commented Feb 22, 2023

I noticed the naming discrepancy too, but wasn't sure if that was the problem.
But it does explain that i cant actually find the malicious code, since that only exists in the deleted piwik-pro-angular-tracking package.

I'm also looking into the source of each of the npm published versions, in everyone the name in the package.json is correct: "name": "@piwikpro/ngx-piwik-pro",.
Only the first version v0.0.9, i havent been able to check, since NPM is still building the file list https://www.npmjs.com/package/@piwikpro/ngx-piwik-pro/v/0.0.9?activeTab=explore

@andrii-lundiak
Copy link

andrii-lundiak commented Feb 22, 2023

@koesper FYI :) => #13

@koesper
Copy link
Author

koesper commented Feb 22, 2023

I'm trying to reproduce the steps, gimme a minute

@andrii-lundiak
Copy link

andrii-lundiak commented Feb 22, 2023

I also executed npm ci and no audit warnings:

image

I'm on Node v16, npm v8.

@andrii-lundiak
Copy link

andrii-lundiak commented Feb 22, 2023

@koesper Also try to:

npm i [email protected]:andrii-lundiak/ngx-piwik-pro.git#4ee9734b44ab3ee751593f595198796fc17aa389

meaning that you will get package AFTER my change in scope of my PR #13.

U know.

But that branch is based on master and on NodeJS v10 API, so you Node v18 will BREAK lock file. You have to have OLD nodejs to be on the same conditions as package itself.

As you see I do have OK again:
image

@koesper
Copy link
Author

koesper commented Feb 22, 2023

I think i'm going crazy, i've been able to reproduce this from scratch multiple times, but now my audit is not giving errors anymore..

what i did was:

npx @angular/cli@13 new myProject
cd myProject
npm i @piwikpro/ngx-piwik-pro
vi package.json #to check if it was actually installed
npm audit

and then i got that error.
i forced angular-cli to use a old version, to get around the versioning problem, but i've also done it with 'regular' v15, or with an ancient v9.
sometimes i needed to use --force or --legacy-peer-deps, to force it to install.

perhaps cleaning my npm cache triggered it to reload the audit data? i'm very confused, because now i cant reproduce it anymore.

But i do feel that your pullrequests should solve the problems!
#11 to fix the versioning
#13 to get rid of all references to the package-name-squatting malware

Thanks for going on this adventure with me @andrii-lundiak ! I've learned a lot today about NPM security

@andrii-lundiak
Copy link

andrii-lundiak commented Feb 22, 2023

@koesper First of all --force is a sign that Angular packages DO MISMATCH OR some packages DO have legacy dependencies. It's not ur problem for sure :) As soon as u have to use --force AFTER installing PIWIK package, that is PIWIK's issue for 100%. Calm down :)

@andrii-lundiak
Copy link

andrii-lundiak commented Feb 22, 2023

@koesper Second of all, if you use npx for basic setup, I am 1) not sure what version npx u have locally and 2) what version of npm npx would pick up. I personally AVOID using npx. And clean Node / npm versions for Angular v13 Project setup should be at least 100% same as for PIWIK package. To be clean.

@andrii-lundiak
Copy link

andrii-lundiak commented Feb 22, 2023

@koesper And 3) sure, if u did npm cache clean --force than you may have got rid of cached version of SOME package, not necessary of PWIK or ng-xyz... Just some... Then after fresh npm install or npm ci (which REMOVES node_modules) npm audit may regenerate new result.

@koesper koesper closed this as completed Jul 16, 2024
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

2 participants