-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Closed
Description
One of the default HTTP redirect checks, located here, is intended to "approve" a redirect if the destination the same host or at least a domain:
function (url, host, port)
local hostname = stdnse.get_hostname(host)
if ( hostname == host.ip and host.ip == url.host.ip ) then
return true
end
local domain = hostname:match("^[^%.]-%.(.*)") or hostname
local match = ("^.*%s$"):format(domain)
if ( url.host:match(match) ) then
return true
end
return false
end,There are several issue with this implementation:
url.hostis a string, not a table withipas a member. As a result the condition to "approve" a redirect when the IP addresses match (host.ip == url.host.ip) would never fire.- The redirect is not immediately rejected when the current hostname and the destination are different IP addresses. The code instead proceeds with a domain-matching logic, which effectively means that two IP addresses are considered a match if they differ at most in the first octet while the remaining three are identical.
- The domains should not be compared case-sensitively.
- The domain-matching logic has a weakness in that it does not treat the current location and the destination symmetrically. As an example, it "approves" a redirect from
www.foo.comtowww.test.foo.combut not the opposite direction. - The logic approves unwarranted redirects, such as
foo.comtobar.com(because they share the most specific domain portion, which in this case is.com) orwww.bank.comtomy.fakebank.com(because the second name ends withbank.com) - The domain matching is using the domain suffix as a Lua pattern, which is problematic when the domain contains a dash or, quite naturally, a dot. As an example,
f-bomb.orgcan be redirected tobomb.comorfoo.bar.rink.nettobardrink.net.
The following patch resolves these issues:
--- a/nselib/http.lua
+++ b/nselib/http.lua
@@ -1485,15 +1485,12 @@
-- Check if the location is within the domain or host
function (url, host, port)
local hostname = stdnse.get_hostname(host)
- if ( hostname == host.ip and host.ip == url.host.ip ) then
- return true
+ if hostname == host.ip then
+ return url.host == hostname
end
- local domain = hostname:match("^[^%.]-%.(.*)") or hostname
- local match = ("^.*%s$"):format(domain)
- if ( url.host:match(match) ) then
- return true
- end
- return false
+ local srcdomain = (hostname:match("%..+%..+") or hostname):lower()
+ local dstdomain = (url.host:match("%..+%..+") or url.host):lower()
+ return srcdomain == dstdomain
end,
-- Check whether the new location has the same port numberThe following caveats apply:
- Domain portions must match exactly. A redirect from
www.foo.comtowww.test.foo.comwill be rejected in either direction. - The domain match must be at least two levels deep. Sharing a TLD is not good enough.
- ccTLDs are not treated as such.
Please let me know if you have any questions or concerns. Otherwise I will commit the patch in a few weeks.
Metadata
Metadata
Assignees
Labels
No labels