PiwikPRO / ngx-piwik-pro

Dedicated Piwik PRO library that helps with implementing Piwik PRO Tag Manager and the Piwik PRO tracking client in Angular 8+ applications.
MIT License
3 stars 3 forks source link

npm audit gives malware warning? #12

Closed koesper closed 3 months ago

koesper commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

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 commented 1 year ago

@koesper FYI :) => https://github.com/PiwikPRO/ngx-piwik-pro/pull/13

koesper commented 1 year ago

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

andrii-lundiak commented 1 year ago

I also executed npm ci and no audit warnings:

image

I'm on Node v16, npm v8.

andrii-lundiak commented 1 year ago

@koesper Also try to:

npm i git@github.com:andrii-lundiak/ngx-piwik-pro.git#4ee9734b44ab3ee751593f595198796fc17aa389

meaning that you will get package AFTER my change in scope of my PR https://github.com/PiwikPRO/ngx-piwik-pro/pull/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 commented 1 year ago

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 commented 1 year ago

@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 commented 1 year ago

@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 commented 1 year ago

@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.