SSRF
keworr opened this issue · 17 comments
Describe
There is a way to bypass your regex to validate private & local networks.
If we use http://127.0.0.1/ or http://localhost/ to link preview, we don't see it (Error: link-preview-js did not receive a valid a url or text), but if we use a domain that resolved to 127.0.0.1, we can. For example: localtest.me resolved to 127.0.0.1 (localhost), i.e. If you 'curl localtest.me', you'll see your localhost.
Similarly we can read any other private & local address, any port.
To Reproduce
Steps to reproduce:
- Find domain that resolved to private address with reverse ip lookup or use localtest.me (127.0.0.1) or devhead.net (127.0.0.1 + 192.168.1.1 + 192.168.0.1).
- Write it to getLinkPreview.
- Done. You see your local domain.
Screenshots
Which version are you using? At some point, someone submitted a PR to patch redirections, by default it should not follow any.
I don't quite get what is the issue though... these sites seem to register the loopback address on DNS level. Even curl returns the data of the directory, then it also means that curl has an SSRF vulnerability?
The regex is not a security feature, it's only a way to detect links in the text. As stated, the library does not follow redirections by default.
I really don't understand what you mean. In that issue, the author submitted a PR not to follow redirects, which is now the default behavior. The same author complained that it is possible to pass the regex with a localhost url, like I said, the regex is not meant to be a security feature, it is only there for link detection. Once the url is passed to the fetch library, there is no way to re-validate the url (even if the regex was meant to do that, which it is not) . On the other hand, I already showed you that curl also has this same behavior, because the domains you send use the localhost address on a DNS level, so that means (as far as I can see) that there is no SSRF vulnerability, this is just the expected behavior.
Redirects were disabled in that issue because it allows to read local hosts, right?
And my way also allows to read local hosts?
idk what is different
Yes, curl by default allow same, also curl allow requests as curl file:///etc/passwd by default, SMB requests, FTP, etc. Also e.g. any browsers allow redirects by default, but you disabled it.
I just saw that issue and thought that SSRF is sensitive here, if not - ok.
redirects were disallowed not because localhost, but because redirections are by default insecure. Your way allows to reach localhost, not because the library is doing something wrong, but because the domains you provided loopback to localhost on the DNS level, they are not intercepting the request and redirecting, they are straight up returning 127.0.0.1 when their DNS address is resolved. There is nothing I can do about that.
@ospfranco first of all i install lastest version of package for testing my local machine and cant see followRedirects option in source could there be a problem with npm? can you check please
Second of all i agree with @keworr this vulnerability is called DNS pinning (or DNS rebinding) and it allow to bypass all control mechanism on DNS level but library has proxy option and the purpose of proxy is to fix such DNS level problems and vulnerabilities. But if we want the library to prevent such DNS level attacks as well we can fix this by dns lookup before make request
hi @AbdurrahmanA, you are right I published a new version of the package, probably some cache was left on my machine when I published one of the newer versions. Thanks a lot!
Thanks a lot for the explanation, that was what I needed. This StackOverflow answer seems to suggest there is no way to resolve the DNS address from JavaScript, seems like the solution is to self-host a proxy?
Actually if we added that kind of option to library it would be great i would love have that option. We can easily do nslookup for server side, for client side we can use https://dns.google/resolve?name=localtest.me or that option wont be available for client side. By the way this library should not be in client side it wil get lots of CORS error 😁
If you mean hardcoding a DNS service, not a fan (and especially google's), but maybe an option to pass a DNS resolver if one chooses to do so.
The library will face CORS errors on websites, on React Native, Cordova, etc it works just fine
We can easily set DNS resolver for Node js side, but if we do this using native dns library, will it be a problem for client-side users? otherwise we should make an online dns resolver option right?
I'm still not sure if the DNS resolver should always be there, just from a latency point of view, it might affect people who have already deployed this. But just passing a function that resolves the host might be enough for us to check against common attacks?
getLinkPreview('some text with a link', {
// Must resolve the url host, internally checks for loopbacks and common SSRF attacks
resolveDNSHost: (url: string) => Promise<string>
})
On the other hand, I don't think this would be an issue on mobiles. Almost nobody has a server running on the phone for the loopback address to be considered dangerous. Maybe conditionally importing/requiring the DNS resolver on node is enough...
Sorry for late response, Yes we just need the DNS resolver on node can we do that and also, we need to warn people about common SSRF vulnerabilities and make sure people understand what this library does.
Most people won't care 🤷♂️ they barely read the CORS warning. But I'll try to find some time in the coming weeks to add the option, otherwise PRs are welcome
Great to hear that, i could also do it if i have time, i will inform you 👌