CVE-2024-29415: ip SSRF improper categorization in isPublic

[dalibor]

Hello - could we remove @msimerson/stun package as an optional dependency for this package?

It includes ip package that has an open CVE and does not seem to be maintained anymore: GHSA-78xj-cgh5-2h22?

$ npm audit
# npm audit report

ip  *
Severity: high
ip SSRF improper categorization in isPublic - https://github.com/advisories/GHSA-2p57-rm9w-gvfp
No fix available
node_modules/ip
  @msimerson/stun  *
  Depends on vulnerable versions of ip
  node_modules/@msimerson/stun

2 high severity vulnerabilities

[msimerson]

could we remove @msimerson/stun package as an optional dependency for this package?

No. Haraka is often installed behind NATs and needs the ability to discover its public IP. It's an optional dependency, you can use option flags to skip optional deps.

It includes ip package...

Then @msimerson/stun needs to be updated. There's a lot of higher priority items above that on my list.

[dalibor]

Looks like stun is not an optional dependency since the following test fails:

  is_local_host
undefined
Please install stun: "npm install -g stun"
    1) self ip


  0 passing (2s)
  1 failing

  1) is_local_host
       self ip:
     Error: Timeout of 2000ms exceeded. For async tests and hooks, ensure "done()" is called; if returning a Promise, ensure it resolves.
      at listOnTimeout (node:internal/timers:569:17)
      at process.processTimers (node:internal/timers:512:7)

@msimerson I wonder if get_public_ip could be extracted into a separate library and included only where it's needed (one example being geoip plugin) or alternatively we can just replace the "ip" package in https://github.com/msimerson/stun with neoip or any of the other alternatives to fix the CVE?

[msimerson]

Looks like stun is not an optional dependency since the following test fails

Where the dependency is listed in package.json determines what kind of dependency it is.

alternatively we can just replace the "ip" package in https://github.com/msimerson/stun with indutny/node-ip#150 (comment) or any of the other alternatives to fix the CVE?

This is the avenue that I would pursue. It looks like the ip library is only used in two places in stun, and could be easily replaced with ipaddr.js, which this module already depends on.

[dalibor]

This is the avenue that I would pursue. It looks like the ip library is only used in two places in stun, and could be easily replaced with ipaddr.js, which this module already depends on.

Sounds great, thank you!