search

vin01 / bogus-cves

Got a bogus CVE someone got for a non-vuln? Please share here!
23 stars 1 forks source link

Add CVE-2023-42282 #6

Closed vin01 closed 3 months ago

vin01 commented 3 months ago

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.

ouuan commented 3 months ago

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

ouuan commented 3 months ago

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 commented 3 months ago

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 commented 3 months ago

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 https://github.com/indutny/node-ip/issues/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.