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

Add CVE-2023-42282 #6

Merged
merged 1 commit into from
Jul 24, 2024
Merged

Add CVE-2023-42282 #6

merged 1 commit into from
Jul 24, 2024

Conversation

vin01
Copy link
Owner

@vin01 vin01 commented Jul 24, 2024

The impact and actual exploitability of this bug are heavily overstated.

@indutny made an excellent comment here regarding this and clearly was bombarded with unnecessary noise as a result of this silly CVE being published:

I'm very curious how an untrusted input could end up being passed into ip.isPrivate or ip.isPublic and then used for verifying where the network connection came from. After all, if you want to prevent access to the service based on the ip address, you would get the ip address from the OS and it will be properly formatted 100% of the times.

@vin01 vin01 merged commit a975681 into main Jul 24, 2024
@vin01 vin01 deleted the chore/add-CVE-2023-42282 branch July 24, 2024 08:55
@ouuan
Copy link

ouuan commented Aug 7, 2024

Please read my comments in that PR. @indutny is unfamiliar with SSRF attacks, and the comment you quoted above clearly shows that.

@ouuan
Copy link

ouuan commented Aug 7, 2024

We tried our best to contact the maintainer through emails, Mastodon, and GitHub, before and after the disclosure, and I have proposed a patch, all without any response from the maintainer. The maintainer instead stated that "I wasn't involved in any step of this process from its beginning" in a thread not subscribed by the CVE reporters. And you are adding this as a bogus CVE without responding to my comments or notifying me. It's just ridiculous.

I read the whole set of relevant posts on Daniel's blog before working on CVE-2023-42282 and I totally agree on the harm of bogus CVEs. But maybe it's worse to have a bogus list of bogus CVEs.

You should learn about not only the harm of bogus CVEs from the curl blogs, but also how curl effectively handles security vulnerabilities with best practices. Please note that the main fault of a bogus CVE is the lack of proper communication.

Sincerely.

@vin01
Copy link
Owner Author

vin01 commented Aug 8, 2024

Hi there, thanks for sharing your perspective. I did read your comments and still came to the same conclusion about exploitability and the benefit of this CVE ID.

As for older "vulnerable" versions, there simply was no validation for an IP address. An IP address like localhost would also result in a SSRF if application used the library that way.

> ip.isPrivate('localhost')
false
> ip.isPrivate('ssh://example.com')
false

Now, at least I wouldn't get CVE IDs for these since it simply is not what the package was even validating. No one should be relying on a simple IP validation of this form to actually prevent SSRF. DNS, TOCTOU, DNS rebinding, many ways exist to bypass it.

Did you encounter any projects which were relying on this functionality for validating user provided IP addresses / URLs to defend against SSRF?

OWASP recommended ip-address seems like the package which is designed for this purpose.

JavaScript: Library ip-address.

It is NOT exposed to bypass using Hex, Octal, Dword, URL and Mixed encoding.

I do see your point about communication with indutny and while I do think it could have been better handled, it always heavily depends on personal situations. It could simply have been filed as a bug and fixed if deemed appropriate or clarified in documentation that this is not the responsibility undertaken by this project. But this is very often the case with open source.

See indutny's point after the exploitation scenario was further clarified: https://fosstodon.org/@indutny/112717658178047083

@ouuan
Copy link

ouuan commented Aug 8, 2024

Thanks for your clarification. I thought that you would not call "verifying where the network connection came from" an "excellent comment" if you knew what SSRF is.

No one should be relying on a simple IP validation of this form to actually prevent SSRF. DNS, TOCTOU, DNS rebinding, many ways exist to bypass it.

It's usually hard to prevent SSRF attacks on the application layer only, and I have mentioned this in indutny/node-ip#150. However, SSRF can be prevented if only public IP addresses are allowed, while domains and private IP addresses are banned (or the IP address of the domain is queried only once). This is possible through isV4Format() and then isPrivate()/isPublic().

I do see your point about communication with indutny and while I do think it could have been better handled, it always heavily depends on personal situations. It could simply have been filed as a bug and fixed if deemed appropriate or clarified in documentation that this is not the responsibility undertaken by this project. But this is very often the case with open source.

I don't mean to blame the maintainer for not responding. My point is that, given this communication situation, the CVE is not a bogus one even if it has an inaccurate assessment of the issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants