feat: node audit script #857

jimcase · 2024-12-08T22:14:07Z

Description

Vulnerabilities Status

1. cross-spawn

Severity: High
Description: Regular Expression Denial of Service (ReDoS) in cross-spawn.
Packages Involved: node_modules/appium-safari-driver/node_modules/cross-spawn
Solution: Override to 7.0.5. Final solution with PR: tapjs/foreground-child#65
Link: GHSA-3xgq-45jj-v275

2. ip

Severity: High
Description: SSRF improper categorization in isPublic.
Packages Involved: ip, @fabianbormann/meerkat, bittorrent-tracker, torrent-discovery, webtorrent, ip-set, load-ip-set
Solution: No patch available
Link: GHSA-2p57-rm9w-gvfp

3. path-to-regexp

Severity: Moderate
Description: Unpatched ReDoS vulnerability in versions prior to 0.1.12.
Packages Involved: path-to-regexp, express
Solution: Override to 0.1.12
Link: GHSA-rhx6-c78j-4q9w

4. validate.js

Severity: Moderate
Description: Regular Expression Denial of Service vulnerability.
Packages Involved: validate.js, @appium/base-driver, @appium/base-plugin, appium, appium-android-driver, appium-uiautomator2-driver, appium-safari-driver, appium-xcuitest-driver
Solution: No patch available
Link: GHSA-rv73-9c8w-jp4c

Audit Script

A Node script that automates the analysis and reporting of project dependency vulnerabilities, utilizing data from audit-results.json and ignored-node-vulnerabilities.json. Integrated into the GitHub Actions pipeline, the script provides a detailed console report that categorizes each vulnerability by severity and whether it is ignored, aiding in swift vulnerability management.

npm run audit

The addition to the GitHub Actions pipeline enhances project security by automating the detection and documentation of potential risks, offering actionable insights through an organized summary of findings.

Include audit script in gh-verify-pr.yaml

    - name: Audit Dependencies
      run: npm run audit

Overriding internal packages

For some reason the overried is not being applied in the package.json. Docs

Example

Get package path:

cd node_modules/appium-safari-driver
npm list cross-spawn

Nested dependencies:

cf-identity-wallet/node_modules/appium-safari-driver
└─┬ archiver-utils@5.0.2 extraneous
  └─┬ glob@10.4.5
    └─┬ foreground-child@3.3.0
      └── cross-spawn@7.0.3

Package.json update:

"overrides": {
    "appium-safari-driver": {
      "archiver-utils": {
        "glob": {
          "foreground-child": {
            "cross-spawn": "^7.0.5"
          }
        }
      }
    },
    "express": {
      "path-to-regexp": "0.1.12"
    }
  },

Checklist before requesting a review

Issue ticket number and link

This PR has a valid ticket number or issue: DTIS-1467

iFergal · 2024-12-10T14:09:32Z

package.json

+      }
+    },
+    "express": {
+      "path-to-regexp": "0.1.12"


Express is part of the test credential server, not here. Not too important if there's vulnerabilities there but I guess better to not have dependabot notifications.

Based on the docs, this rule applies to all path-to-regexp(any level) after express.

In root directory, run:

npm list path-to-regexp

Paths related to path-to-regexp@0.1.10

... ├─┬ appium-uiautomator2-driver@3.9.2 │ └─┬ appium@2.13.1 │ └─┬ @appium/base-driver@9.13.1 │ ├─┬ express@4.21.1 overridden │ │ └── path-to-regexp@0.1.10 overridden │ └── path-to-regexp@8.2.0 ├─┬ appium-xcuitest-driver@7.33.0 │ └─┬ appium-remote-debugger@12.1.4 │ └─┬ @appium/base-driver@9.13.1 │ ├─┬ express@4.21.1 overridden │ │ └── path-to-regexp@0.1.10 overridden │ └── path-to-regexp@8.2.0 ...

Now that I see it better, there it indicates that they are overriden, but I don't know why it is still with version 0.1.10.

Ah appium has express. Looking at that it seems like it's both a dependency of express and @appium/base-driver so you would need another override, or maybe do it at a higher level.

iFergal · 2024-12-10T14:13:52Z

package.json

+      "archiver-utils": {
+        "glob": {
+          "foreground-child": {
+            "cross-spawn": "^7.0.5"


From a quick search, I think it should just be appium-safari-driver -> cross-spawn and remove the archiver-utils -> glob -> foreground-child. Which would align with node_modules/appium-safari-driver/node_modules/cross-spawn

Yes, I tried that also but it did not work

This is the used library between all packages:

/cf-identity-wallet ├─┬ @capacitor/assets@3.0.5 │ └─┬ @trapezedev/project@7.1.3 │ └── cross-spawn@7.0.6 deduped ├─┬ @capacitor/cli@6.2.0 │ └─┬ @ionic/utils-subprocess@2.1.11 │ └── cross-spawn@7.0.6 deduped ├─┬ @wdio/cli@9.4.1 │ └─┬ execa@9.5.2 │ └── cross-spawn@7.0.6 deduped ├─┬ @wdio/cucumber-framework@9.3.1 │ └─┬ glob@10.4.5 │ └─┬ foreground-child@3.3.0 │ └── cross-spawn@7.0.6 deduped ├─┬ appium-chromium-driver@1.4.5 │ └─┬ appium-chromedriver@6.1.7 │ └─┬ @appium/support@5.1.8 │ └─┬ glob@10.4.5 │ └─┬ foreground-child@3.3.0 │ └── cross-spawn@7.0.6 ├─┬ appium-safari-driver@3.5.18 overridden │ └─┬ archiver-utils@5.0.2 extraneous overridden │ └─┬ glob@10.4.5 │ └─┬ foreground-child@3.3.0 overridden │ └── cross-spawn@7.0.3 ├─┬ appium-uiautomator2-driver@3.9.2 │ ├─┬ appium-adb@12.7.2 │ │ └─┬ @appium/support@6.0.0 │ │ └─┬ glob@10.4.5 │ │ └─┬ foreground-child@3.3.0 │ │ └── cross-spawn@7.0.6 deduped │ └─┬ appium@2.13.1 │ └─┬ cross-env@7.0.3 │ └── cross-spawn@7.0.6 ├─┬ appium-xcuitest-driver@7.33.0 │ └─┬ appium-remote-debugger@12.1.4 │ └─┬ glob@10.4.5 │ └─┬ foreground-child@3.3.0 │ └── cross-spawn@7.0.6 ├─┬ cross-env@7.0.3 │ └── cross-spawn@7.0.6 ├─┬ eslint@8.57.1 │ └── cross-spawn@7.0.6 deduped ├─┬ jest@29.7.0 │ └─┬ @jest/core@29.7.0 │ └─┬ jest-changed-files@29.7.0 │ └─┬ execa@5.1.1 │ └── cross-spawn@7.0.6 deduped ├─┬ lint-staged@15.2.11 │ └─┬ execa@8.0.1 │ └── cross-spawn@7.0.6 deduped ├─┬ webpack-cli@5.1.4 │ └── cross-spawn@7.0.6 deduped └─┬ webpack-dev-server@4.15.2 └─┬ default-gateway@6.0.3 └─┬ execa@5.1.1 └── cross-spawn@7.0.6 deduped

The only dependency affected is:

├─┬ appium-safari-driver@3.5.18 overridden │ └─┬ archiver-utils@5.0.2 extraneous overridden │ └─┬ glob@10.4.5 │ └─┬ foreground-child@3.3.0 overridden │ └── cross-spawn@7.0.3

OK but the other packages look ok - have you tried on a fresh install after wiping node_modules? npm/cli#5850 <- seems to be issues

Yes, I tried but it did not work

iFergal · 2024-12-10T14:14:50Z

package.json

As per the ticket, I'd like to add npm audit. I wonder if there's a way we can add this to the pipeline but ignore the ones with no known fix (or just these specific ones and revisit)

I haven't looked deep into the vulnerabilities either, could be that they don't apply to us. Need to check.

Okey sure, this is still missing.

I added the script to handle ignored vulnerabilities and also to the gh pipeline

jimcase added 9 commits December 8, 2024 23:07

feat: upgrade lint-staged

4d1482c

feat: upgrade body-parser

722f639

feat: upgrade cardano-peer-connect

17e7021

fix: run npm ci and fix package-lock.json

0f3e367

fix: upgrade @capacitor/cli

5cc15a6

fix: upgrade appium

8d54ba5

feat: remove unused overrides from package.json

bb9901b

override appium-safari-driver

a8f0aba

fix: path-to-regexp vuln

7ea6ba8

jimcase self-assigned this Dec 10, 2024

jimcase marked this pull request as ready for review December 10, 2024 10:12

jimcase requested a review from iFergal as a code owner December 10, 2024 10:12

fix: package-lock

be1e7f4

iFergal reviewed Dec 10, 2024

View reviewed changes

jimcase added 16 commits December 10, 2024 16:12

fix: update package-lock.json

b3cc6a9

fix: merge develop

2d0f85b

feat: add audit filter

d998f40

fix: test ignore all

a40abbd

fix: check if audit-results.json exists

e41035c

fix: check existing files

88e8fc7

fix: pipeline Audit Dependencies

be9c9bc

fix: testing audit-filter

b1ec54e

fix: testing pipeline output

922873c

fix: testing pipeline output

35dfe8c

fix: testing pipeline output

0216bf0

fix: testing pipeline output

590dbca

feat: add script to package.json

acf972d

feat: format and add guide

df0e36f

fix: format output

7f3e791

fix: format output

3b99998

jimcase added 4 commits December 10, 2024 22:36

fix: format output

0180dfc

feat: expand details

dd19d71

fix: ignored-node-vulnerabilities.json

6603dcf

fix: clean ga audit

b24a863

jimcase changed the title ~~Feature/fix vulnerabilities~~ feat: node audit script Dec 10, 2024

iFergal force-pushed the develop branch from b15bd4d to 5d6cdc1 Compare December 13, 2024 18:25

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: node audit script #857

feat: node audit script #857

jimcase commented Dec 8, 2024 •

edited

Loading

iFergal Dec 10, 2024

jimcase Dec 10, 2024

iFergal Dec 10, 2024 •

edited

Loading

iFergal Dec 10, 2024

jimcase Dec 10, 2024

jimcase Dec 10, 2024

iFergal Dec 10, 2024 •

edited

Loading

jimcase Dec 11, 2024

iFergal Dec 10, 2024

jimcase Dec 10, 2024

jimcase Dec 11, 2024

feat: node audit script #857

Are you sure you want to change the base?

feat: node audit script #857

Conversation

jimcase commented Dec 8, 2024 • edited Loading

Description

Vulnerabilities Status

1. cross-spawn

2. ip

3. path-to-regexp

4. validate.js

Audit Script

Overriding internal packages

Example

Checklist before requesting a review

Issue ticket number and link

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iFergal Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

iFergal Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jimcase commented Dec 8, 2024 •

edited

Loading

iFergal Dec 10, 2024 •

edited

Loading

iFergal Dec 10, 2024 •

edited

Loading