-
-
Notifications
You must be signed in to change notification settings - Fork 130
SSRF #115
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
Comments
Which version are you using? At some point, someone submitted a PR to patch redirections, by default it should not follow any. |
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. |
But you accepted same SSRF here - #105? |
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? Yes, curl by default allow same, also curl allow requests as 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 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 |
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 |
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?
|
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 👌 |
Uh oh!
There was an error while loading. Please reload this page.
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:
Screenshots

The text was updated successfully, but these errors were encountered: