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: ip dependency has vulnerability #422

Closed
wants to merge 2 commits into from
Closed

Conversation

nimirium
Copy link

@nimirium nimirium commented Dec 9, 2024

ip dependency has a vulnerability: GHSA-2p57-rm9w-gvfp.
It is used by proxy-agent version 6.3.0.
Bump proxy-agent to 6.5.0 which doesn't use ip.

@parfeon parfeon self-assigned this Dec 12, 2024
@parfeon parfeon added status: rejected This issue is considered rejected. It will not be worked on. priority: low This PR should be reviewed after all high and medium PRs. type: fix This PR contains fixes to existing features. labels Dec 12, 2024
@parfeon
Copy link
Contributor

parfeon commented Dec 12, 2024

Hello.

We've double checked dependency tree for proxy-agent and there is no dependency against ip package:

npx npm-remote-ls proxy-agent@6.3.0 | grep ip                                                                                                                                                                                                             25s 11:33:20
(node:2196) [DEP0040] DeprecationWarning: The `punycode` module is deprecated. Please use a userland alternative instead.
(Use `node --trace-deprecation ...` to show where the warning was created)
   │  ├─ @tootallnate/quickjs-emscripten@0.23.0
   │     └─ ip-address@9.0.5
   │  │  │  │  ├─ strip-ansi@6.0.1
   │  │  │  │     ├─ strip-ansi@6.0.1
   │  │  │  │  ├─ strip-ansi@6.0.1
   │  │  │  ├─ strip-json-comments@3.1.1
   │     ├─ strip-ansi@6.0.1
   │     │  ├─ strip-bom@4.0.0
   │     │  │  └─ strip-ansi@6.0.1
   │     │  ├─ @babel/plugin-syntax-typescript@7.25.9
   │     │  ├─ strip-ansi@6.0.1
   │     │  │  ├─ strip-final-newline@2.0.0
   ├─ typescript@5.7.2

So we will close this PR without merge.

@parfeon parfeon closed this Dec 12, 2024
@nimirium
Copy link
Author

Hi @parfeon, thanks for replying. I'm afraid printing the tree the way you did doesn't cover my case, since it's not a direct dependency.

proxy-agent@6.3.0 requires pac-proxy-agent@^7.0.0 (link)
pac-proxy-agent@7.0.0 requires pac-resolver@^7.0.0 (link)
pac-resolver@7.0.0 requires ip@^1.1.8 (link)

@nimirium
Copy link
Author

yarn why ip 
yarn why v1.22.22
[1/4] 🤔  Why do we have the module "ip"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
[4/4] 🚡  Calculating file sizes...
=> Found "ip@1.1.9"
info Reasons this module exists
   - "_project_#pubnub#proxy-agent#pac-proxy-agent#pac-resolver" depends on it
   - Hoisted from "_project_#pubnub#proxy-agent#pac-proxy-agent#pac-resolver#ip"

@parfeon
Copy link
Contributor

parfeon commented Dec 12, 2024

@nimirium I've removed package-lock.json and reinstalled dependencies, after which package-lock.json contains these:

  • proxy-agent (6.4.0)
  • pac-proxy-agent (7.0.2)
  • pac-resolver (7.0.1) (download) which doesn't require ip package at all. This is pac-resolver dependencies.

@nimirium
Copy link
Author

So you're basically saying "it works on my machine" 😄

proxy-agent@^6.3.0 may install version 6.3.0 for some, and a higher version for others. If you allow proxy-agent v6.3.0, you allow a vulnerability. Please patch this.

Meanwhile I'm using a selective dependency resolution in my package.json (in case someone else needs this):

"resolutions": {
  "pubnub/proxy-agent": "^6.5.0",
},

@parfeon
Copy link
Contributor

parfeon commented Dec 12, 2024

I just tried to use yarn within package repo, and it still follows semver rules which let it pull out proxy-agent which fits ^ requirement:

yarn why ip                                                                                                                                                                                                                                         1m 23s 22:30:53
yarn why v1.22.22
[1/4] 🤔  Why do we have the module "ip"...?
[2/4] 🚚  Initialising dependency graph...
[3/4] 🔍  Finding dependency...
error We couldn't find a match!
✨  Done in 0.10s.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
priority: low This PR should be reviewed after all high and medium PRs. status: rejected This issue is considered rejected. It will not be worked on. type: fix This PR contains fixes to existing features.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants