Skip to content

Broken HTTP redirect check for host/domain #829

@nnposter

Description

@nnposter

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.host is a string, not a table with ip as 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.com to www.test.foo.com but not the opposite direction.
  • The logic approves unwarranted redirects, such as foo.com to bar.com (because they share the most specific domain portion, which in this case is .com) or www.bank.com to my.fakebank.com (because the second name ends with bank.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.org can be redirected to bomb.com or foo.bar.rink.net to bardrink.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 number

The following caveats apply:

  • Domain portions must match exactly. A redirect from www.foo.com to www.test.foo.com will 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

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions