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

[Security] cli-columns and cli-table3 have dependencies to vulnerable packages #3785

Closed
1 task done
akaraman85 opened this issue Sep 22, 2021 · 21 comments
Closed
1 task done
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release Security security related

Comments

@akaraman85
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

Current Behavior

Security scans fail do to high warning of a security vulnerability in ansi-regex.

Expected Behavior

Security scan pass.

Steps To Reproduce

We use twistlock to do vulnerability detection, which relies on NVD to get vulnerability data.

The issue can be found here, https://nvd.nist.gov/vuln/detail/CVE-2021-3807 and here, https://snyk.io/vuln/npm:ansi-regex.

Environment

  • OS: Mac 11.5.2
  • Node: v12.21.0
  • npm: 7.21.1
@akaraman85 akaraman85 added Bug thing that needs fixing Needs Triage needs review for next steps Release 7.x work is associated with a specific npm 7 release labels Sep 22, 2021
@lukekarrys lukekarrys added Security security related Priority 1 high priority issue and removed Needs Triage needs review for next steps labels Sep 23, 2021
@Izook
Copy link

Izook commented Oct 7, 2021

Also running into this same issue. Looking at the attached PR will this issue not be addressed until version 8 since cli-columns doesn't support Node 10?

@ckcr4lyf
Copy link

ckcr4lyf commented Oct 7, 2021

Also ran into this due to npm itself being dependent on it. Decided to add it to the CVE ignore list after confirming no other occurrences of the vulnerable issue

@thoger
Copy link

thoger commented Oct 13, 2021

Note that the ansi-regex upstream PR and commit for CVE-2021-3807 are:

chalk/ansi-regex#37
chalk/ansi-regex@8d1d7cd

Also note that versions 3.0.0 - 5.0.0 and 6.0.0 are affected, issue was fixed in 5.0.1 and 6.0.1.

This report mentions ansi-regex version as bundled with current npm version 7, 8, and latest. However, version 6, that is bundled with current nodejs versions, has even more copies of ansi-regex. Here is the current status:

latest and v8.0.0:
./node_modules/string-width/node_modules/ansi-regex/package.json: "version": "3.0.0", -- affected
./node_modules/ansi-regex/package.json: "version": "2.1.1", -- notaffected
./node_modules/cli-table3/node_modules/ansi-regex/package.json: "version": "5.0.0", -- affected
./node_modules/cli-columns/node_modules/ansi-regex/package.json: "version": "5.0.1", -- fixed

v7.24.2:
./node_modules/string-width/node_modules/ansi-regex/package.json: "version": "3.0.0", -- affected
./node_modules/ansi-regex/package.json: "version": "2.1.1", -- notaffected
./node_modules/cli-table3/node_modules/ansi-regex/package.json: "version": "5.0.0", -- affected

v6.14.15
./node_modules/wrap-ansi/node_modules/ansi-regex/package.json: "version": "4.1.0" -- affected
./node_modules/string-width/node_modules/ansi-regex/package.json: "version": "3.0.0" -- affected
./node_modules/ansi-regex/package.json: "version": "2.1.1", -- notaffected
./node_modules/yargs/node_modules/ansi-regex/package.json: "version": "4.1.0" -- affected
./node_modules/cliui/node_modules/ansi-regex/package.json: "version": "4.1.0" -- affected

The above covers versions that can be found in the respective git tags or branches.

@panayiod44
Copy link

Is there any plan to fix this in npm v6 ?

@f2404
Copy link

f2404 commented Nov 8, 2021

Hello,
Following this bug report: nodejs/docker-node#1574 (comment)

npm 8.1.0 continues to install vulnerable versions of ansi-regex package - namely, @5.0.0, @2.1.1, and @3.0.0 - which results in the base Docker image being flagged by container security software.

Would it be possible to upgrade all dependencies to use ansi-regex@5.0.1?
Thanks!

@guomeng306
Copy link

Hello,
We got same issue,too.
nodejs team asked us to open issue here.
nodejs/node#40853
Thanks a lot.

@skhushboo-vm
Copy link

Hello Team,

Can you please upgrade npm to use fixed ansi-regex?
Also, kindly inform if any timelines are there for this?

Thanks,
Khushboo Soni
VMware

@psandeep09
Copy link

We got same issue.

image

image

@dennismeister93
Copy link

Bildschirmfoto 2021-11-26 um 10 25 51

Same here with using base docker image node:16-alpine

@obanawal
Copy link

Hi npm Team,

Kindly let us know the timeline for fixing this issue.

Thanks,
Omkar
Oracle

@nachtwerk
Copy link

nachtwerk commented Jan 4, 2022

Giving this another bump as this issue is persisting in our CI pipeline's security scan. Any attention to fixing this will make a lot of people happy ❤️

PS. We use trivy for our security scans

@hi-artem
Copy link

hi-artem commented Jan 6, 2022

For anyone blocked by this issue. Don't use NPM in your final container image.

Here is an example of Dockefile with multistage build based on Alpine:

# Build stage
FROM node:16-alpine3.15 as build

# Install dependencies
WORKDIR /
COPY package-lock.json .
COPY package.json .
RUN npm ci --production

# Final stage
FROM alpine:3.15 as final

# Upgrade APK and install nodejs
RUN apk --no-cache add --upgrade nodejs~16

# Setup application
RUN mkdir -p /app/simple-server
WORKDIR /app/simple-server
COPY . .
COPY --from=build node_modules node_modules

# Run App
ENTRYPOINT ["node", "index.js"]

This will make your scanners happy 😊

@f2404
Copy link

f2404 commented Jan 6, 2022

For anyone blocked by this issue. Don't use NPM in your final container image.

We ended up uninstalling npm from the final image:

# Don't need npm during runtime
RUN npm uninstall npm -g

@nachtwerk
Copy link

@hi-artem Thanks, this is the solution we've opted for

@MartinFalatic
Copy link

It would be good to see high vulnerabilities in NPM fixed in a timely fashion, whether or not some deployments can hack NPM out of the equation.

@ljharb
Copy link
Collaborator

ljharb commented Jan 6, 2022

@MartinFalatic "a CVE exists" does not mean "a vulnerability exists". Most CVEs are false positives, and unless these vulnerabilities are exploitable via npm, it's not actually a vulnerability.

An actual vulnerability is a concern - a false positive CVE warning should not be. (i'm not making any assertions about these CVEs/vulnerabilities specifically, just speaking in general terms)

@hi-artem
Copy link

hi-artem commented Jan 6, 2022

@MartinFalatic the thing here is how often do you need to have NPM in your final image? IHMO it is not a hack to remove it, but a good practice of not installing development dependencies in your production environment. NPM itself is the development dependency.

@wraithgar
Copy link
Member

Closed in v8.5.2 thanks @lukekarrys

@franher
Copy link

franher commented Mar 15, 2022

I have verified that updating globally to the latest npm version (v8.5.4) on the Dockerfile also does the trick. The security vulnerabilities disappeared.

FROM node:16-alpine3.15
...
RUN npm -g npm@8.5.4
...

@vanta
Copy link

vanta commented Aug 17, 2022

@wraithgar Is it possible to backport it to version 6.x?

@c3ivodujmovic
Copy link

c3ivodujmovic commented Sep 15, 2022

The earlier assessments of fix incompatibility with npm 6 are probably no longer true, as the fixed ansi-regex is in npm@6.14.17 via yargs@14.2.3/cliui@5.0.0

npm@6.14.17 
├─┬ libnpx@10.2.4
│ └─┬ yargs@14.2.3
│   ├─┬ cliui@5.0.0
│   │ ├─┬ string-width@3.1.0
│   │ │ ├── emoji-regex@7.0.3 deduped
│   │ │ ├── is-fullwidth-code-point@2.0.0
│   │ │ └── strip-ansi@5.2.0 deduped
│   │ ├─┬ strip-ansi@5.2.0
│   │ │ └── ansi-regex@4.1.1
│   │ └─┬ wrap-ansi@5.1.0
│   │   ├── ansi-styles@3.2.1 deduped
│   │   ├─┬ string-width@3.1.0
│   │   │ ├── emoji-regex@7.0.3 deduped
│   │   │ ├── is-fullwidth-code-point@2.0.0
│   │   │ └── strip-ansi@5.2.0 deduped
│   │   └─┬ strip-ansi@5.2.0
│   │     └── ansi-regex@4.1.1

so we just need to upgrade cli-columns to version with strip-ansi@5

├─┬ cli-columns@3.1.2
│ ├─┬ string-width@2.1.1
│ │ ├── is-fullwidth-code-point@2.0.0
│ │ └─┬ strip-ansi@4.0.0
│ │   └── ansi-regex@3.0.0
│ └─┬ strip-ansi@3.0.1
│   └── ansi-regex@2.1.1

@wraithgar @Izook do you agree? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 1 high priority issue Release 7.x work is associated with a specific npm 7 release Security security related
Projects
None yet
Development

No branches or pull requests