Prevent redirecting to local hosts in Oracle

[roman-khimov]

See neo-project/neo#2662 and neo-project/neo-modules#692. Turns out, net/http Client has CheckRedirect exactly for this purpose (but be careful to keep the limit on redirections number).

[roman-khimov]

Also neo-project/neo-modules#694.

[vang1ong7ang]

this comment should be noticed in this topic because of the dns record changes during the 2 dns lookups

neo-project/neo-modules#693 (comment)

[vang1ong7ang]

i suggest to customize http.Client.Transport. i think it's the final solution kkkkkk

[roman-khimov]

The default Transport we have in http.Client caches connections, so I think it'll reuse the same connection it has for example.com if need be (see golang/go#23427 also). At the same time, example.com can always close the connection from its side and that should trigger proper DNS lookup again.

[vang1ong7ang]

neo-go/pkg/services/oracle/request.go

Line 139 in ff60571

r, err := o.Client.Do(httpReq)

inside http.Client.Do, there is a dns lookup. this lookup result may be different with the one in URIValidator. that's the dns rebinding risk.

[vang1ong7ang]

remember the dns record may be controlled by the attacker 🌚

[roman-khimov]

inside http.Client.Do, there is a dns lookup

It's not always the case due to connection caching, but I was thinking more about redirects (the original problem stated) and what can happen during the call processing.

this lookup result may be different with the one in URIValidator

That's true, resolving something in CheckRedirect or URIValidator in general doesn't mean that http.Client will use the same IP, so we can just say that the check was implemented incorrectly right from the beginning.

The way Client/RoundTripper/Transport/Dial things are structured it should be trivial to fix this though, we're TCP-level at Transport.DialContext(), irrespective of when and how DNS was resolved, checking and dialing there is easy.