TooTallNate / proxy-agents

Node.js HTTP Proxy Agents Monorepo
https://proxy-agents.n8.io
871 stars 229 forks source link

Vulnerability for ip package in pac-resolver #280

Closed brickfungus closed 4 months ago

brickfungus commented 4 months ago

https://github.com/advisories/GHSA-78xj-cgh5-2h22

Vulnerability for ip package in pac-resolver. Can that get updated and propagated up through the packages that use it?

viktor-urbanas-qatalog commented 4 months ago

@TooTallNate please help with fixing this high severity issue 🙏 It seems like node-ip repo is dead, maybe worth ditching it and use some alternative

OIRNOIR commented 4 months ago

The ip package is also used by the socks package, which is imported by socks-proxy-agent. It seems like the socks repo might also be dead. Maybe it is worth ditching this one as well?

OIRNOIR commented 4 months ago

Socks seems to use version 2.0.0 of the ip package, which does not appear in the vulnerability report. However, I worry that, since that version does not seem to have been truly released, it is still vulnerable but was not included in the vulnerability report. Installing socks by itself returns no vulnerabilities via pnpm audit. Installing proxy-agent shows one vulnerability for the ip package, and it is the v1.1.8 required by pac-resolver: . > proxy-agent@6.3.1 > pac-proxy-agent@7.0.1 > pac-resolver@7.0.0 > ip@1.1.8 But pnpm why ip returns the following:

dependencies:
proxy-agent 6.3.1
├─┬ pac-proxy-agent 7.0.1
│ ├─┬ pac-resolver 7.0.0
│ │ └── ip 1.1.8
│ └─┬ socks-proxy-agent 8.0.2
│   └─┬ socks 2.7.1
│     └── ip 2.0.0
└─┬ socks-proxy-agent 8.0.2
  └─┬ socks 2.7.1
    └── ip 2.0.0
OIRNOIR commented 4 months ago

So tl;dr: Version 1.1.8 of ip is definitely vulnerable, and i'm not sure whether 2.0.0 is vulnerable, but either way it might be a good idea to transition away from dead packages and packages that require dead packages.

dawidurbanski commented 4 months ago

I added my 2c under the PR.

If socks package won't remove ip dependency upstream, then 2.0.0 will still be used by this repo. Then what's the point of removing ip here.

Version 1.1.8 of ip is definitely vulnerable, and i'm not sure whether 2.0.0 is vulnerable

In the end it does not matter if this package is vulnerable if in this repo you don't use any of the vulnerable functions (which you don't). This makes bumping to 2.0.0 feasible option to get rid of advisory audit info. At least until new advisory report comes out stating 2.0.0 is also affected.

and i'm not sure whether 2.0.0 is vulnerable

I checked the report and I reproduced the vulnerability on 2.0.0 so this version is also affected. It just doesn't affect this package, because vulnerable functions are not used.

dawidurbanski commented 4 months ago

I made a ticket in the socks package just for good measure.

https://github.com/JoshGlazebrook/socks/issues/93

JoshGlazebrook commented 4 months ago

I'll remove it from socks later today and publish a new version.

JoshGlazebrook commented 4 months ago

Published - https://www.npmjs.com/package/socks/v/2.7.3 - Drops ip package as dependency.

dustinaleksiuk commented 4 months ago

Apologies if I'm missing something obvious, but pac-resolver is still using ip 1.1.8. (Edit - I see. The socks package is a different maintainer, but also used under the proxy-agents repo).