[Bug]: The latest version depends on the highly vulnerable ip package

[fyodorio]

Describe the bug

EDIT: New vulnerability is described here: CVE-2024-29415

The vulnerability is described here: GHSA-78xj-cgh5-2h22.

As far as I can see now, the ip package is used only ones in the core-server package here — https://github.com/storybookjs/storybook/blob/ece1fb269cc44f43a5384f986a2b9f48613b0095/code/lib/core-server/src/utils/server-address.ts#L13C1-L13C80

This can easily be swapped with some other package, like for instance https://www.npmjs.com/package/ip-address, the way socks' lib maintainer did here — https://github.com/JoshGlazebrook/socks/pull/94/files.

WDYT guys? Is it a good candidate for a quick fix? I could participate if necessary with the PR, for sure.

To Reproduce

Install any storybook flavour via npm (most of them depend on the vulnerable package through core-server)

System

No response

Additional context

No response

[0xR]

For people searching by CVE number, it is: CVE-2023-42282

[valentinpalkovic]

Just to set some context for the ip vulnerability:

The affected ip.isPublic() method is not used by Storybook. Hence, the vulnerability reported by npm audit doesn't affect Storybook users. We should still consider replacing the package with another one since it isn't maintained anymore.

@fyodorio Please feel free to create a PR with the quick fix :)

[G-Rath]

For those looking for inspiration, sock replaced ip with ip-address and proxy-agents just inlined the methods they were using

[fyodorio]

@valentinpalkovic which branch should I target the fix PR to? Hadn't found any rules/strategies on that. As I can see, most of the PRs are targeted to next but I guess it's the 8th version, right? Or it's OK to do the the same and the update will be backported in a patch to 7th?

[letavocado]

Hey @fyodorio. I guess into the latest-release.

[letavocado]

Oh no! Into next. Based on my research the Release bot generates changes.

For example this PR#25907 commits into next. And release bot takes it as a generated changelog and commits into latest-release PR#25843.

[valentinpalkovic]

Correct! next is the right branch.

[fyodorio]

@valentinpalkovic made the suggestion via #26025, please review, any feedback is welcome, as I'm a first-time contributor here.

[valentinpalkovic]

Please use the 👍 emoji reaction on the initial issue message to upvote the issue, otherwise all core maintainers and participants get notified every time someone posts a „+1“ message, which additionally adds a lot of noise to the thread.

[alilland]

is the plan to add this to 8.0 or will there be the possibility of a 7.X security patch when a solution is chosen 😊

[valentinpalkovic]

I definitely plan to patch this back to 7.6.x!

[grybykm]

I'm replacing ip with https://www.npmjs.com/package/ip-address in some of my packages.
I wanted to share this because it took some time for me to figure what the ip should be replaced with.

[jase88]

Seems to be fixed with 2.0.1: indutny/node-ip#138 (comment)

There is also a PR #26086 (crosslinking #26011)

[valentinpalkovic]

Heads up: The 7.6.17 release contains the fix.

[shajjhusein]

@valentinpalkovic Thanks it's working fine!

[sinnedh]

Unfortunately the vulnerabilty was not fixed completely in ip:2.0.1 and the new CVE-2024-29415 is So reopening this thread and switching to a replacement for the ip package would probably be the way to go.

[xfournet]

#26073 seems the best way to fix the problem. Could be reopened too ?

[j-hannes]

#26073 seems the best way to fix the problem. Could be reopened too ?

That would be great. Or use https://www.npmjs.com/package/address as an alternative solution? CVE-2023-42282 is annoying and not fixed in [email protected].