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

Fix vulnerabilities #6

Merged

Conversation

tjinauyeung
Copy link

@tjinauyeung tjinauyeung commented Aug 9, 2022

Two vulnerabilities were found in the project.

  • One in gulp-delete-lines package, which was not used, so removed it from devDependencies.
  • One is deeply nested and depended on by many packages. We can use the npm overrides field to manually set this one to a specific version

Context

The surface area of the override is limited to the build scripts which lowers the risk on the output.

We can test everything still works by:

  1. Running the build scripts and monitor for thrown errors,
  2. Running the demos after building

npm audit report

glob-parent  <5.1.2
Severity: high
glob-parent before 5.1.2 vulnerable to Regular Expression Denial of Service in enclosure regex - https:/advisories/GHSA-ww39-953v-wcq6
fix available via `npm audit fix --force`
Will install [email protected], which is a breaking change
node_modules/chokidar/node_modules/glob-parent
node_modules/glob-stream/node_modules/glob-parent
  chokidar  1.0.0-rc1 - 2.1.8
  Depends on vulnerable versions of glob-parent
  node_modules/chokidar
    glob-watcher  >=3.0.0
    Depends on vulnerable versions of chokidar
    node_modules/glob-watcher
  glob-stream  5.3.0 - 6.1.0
  Depends on vulnerable versions of glob-parent
  node_modules/glob-stream
    vinyl-fs  >=2.4.2
    Depends on vulnerable versions of glob-stream
    node_modules/vinyl-fs
      gulp  >=4.0.0
      Depends on vulnerable versions of glob-watcher
      Depends on vulnerable versions of vinyl-fs
      node_modules/gulp

lodash.template  <4.5.0
Severity: critical
Prototype Pollution in lodash - https:/advisories/GHSA-jf85-cpcp-j695
No fix available
node_modules/gulp-util/node_modules/lodash.template
  gulp-util  >=1.1.0
  Depends on vulnerable versions of lodash.template
  node_modules/gulp-util
    gulp-delete-lines  *
    Depends on vulnerable versions of gulp-util
    node_modules/gulp-delete-lines

9 vulnerabilities (6 high, 3 critical)

@tjinauyeung tjinauyeung force-pushed the force-versions-for-vulnerable-nested-dependencies branch from 16dee8f to 7f8242b Compare August 9, 2022 06:25
@tjinauyeung tjinauyeung changed the title fix vulnerabilities: override versions for nested dependencies Fix vulnerabilities Aug 9, 2022
@tjinauyeung tjinauyeung self-assigned this Aug 9, 2022
@sam-pitch
Copy link

how can we test that the override doesn't cause any problems before merging? Just try exporting in our app locally with this branch?

@tjinauyeung
Copy link
Author

The surface area of the override is limited to the build scripts which lowers the risk on the output.

We can test everything still works by:

  1. Running the build scripts and monitor for thrown errors,
  2. Running the demos after building

Copy link

@pastafari pastafari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A functional test would be to add a tag on this branch, import it into pitch-app, and see if export continues to work

Copy link

@pastafari pastafari left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes look small enough and contained to gulp, shall we try this out?

@tjinauyeung tjinauyeung merged commit 55c36ab into pitch-main Aug 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants