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

HTTP default port simplification #781

Closed
nnposter opened this Issue Mar 19, 2017 · 2 comments

Comments

Projects
None yet
1 participant
@nnposter

nnposter commented Mar 19, 2017

There are 3 or 4 separate places in http.lua where a scheme is translated into default ports, typically with a code like this:

local port = someport
if not port then
  if scheme == "http" then
    port = 80
  elseif scheme == "https" then
    port = 443
  end
end

The following patch moves this code into a separate function, resulting in substantial code clarity.
(The patch assumes that #766 has been already merged in.)

* Provides a common function for obtaining the default port number for
  a given scheme
--- a/nselib/http.lua
+++ b/nselib/http.lua
@@ -157,16 +157,21 @@
   end
 end
 
+--- Provide the default port for a given scheme.
+local function get_default_port (scheme)
+  local ports = {http=80, https=443}
+  return ports[scheme]
+end
+
 --- Get a value suitable for the Host header field.
 -- See RFC 2616 sections 14.23 and 5.2.
 local function get_host_field(host, port)
   if not host then return nil end
-  local ssl = shortport.ssl(host, port)
-  local pn = port.number
-  if not ssl and pn == 80 or ssl and pn == 443 then
+  local scheme = shortport.ssl(host, port) and "https" or "http"
+  if port.number == get_default_port(scheme) then
     return stdnse.get_hostname(host)
   else
-    return stdnse.get_hostname(host) .. ":" .. pn
+    return stdnse.get_hostname(host) .. ":" .. port.number
   end
 end
 
@@ -1485,14 +1490,7 @@
   function (url, host, port)
     -- port fixup, adds default ports 80 and 443 in case no url.port was
     -- defined, we do this based on the url scheme
-    local url_port = url.port
-    if ( not(url_port) ) then
-      if ( url.scheme == "http" ) then
-        url_port = 80
-      elseif( url.scheme == "https" ) then
-        url_port = 443
-      end
-    end
+    local url_port = url.port or get_default_port(url.scheme)
     if (not url_port) or tonumber(url_port) == port.number then
       return true
     end
@@ -1570,11 +1568,7 @@
     u.path = ((u.path:sub(1,1) == "/" and "" ) or "/" ) .. u.path -- ensuring leading slash
   end
   -- do port fixup
-  if ( not(u.port) ) then
-    if ( u.scheme == "http" ) then u.port = 80
-    elseif ( u.scheme == "https") then u.port = 443
-    else u.port = port.number end
-  end
+  u.port = u.port or get_default_port(u.scheme) or port.number
   if ( not(u.path) ) then
     u.path = "/"
   end
@@ -1668,15 +1662,7 @@
   local port = {}
 
   port.service = parsed.scheme
-  port.number = parsed.port
-
-  if not port.number then
-    if parsed.scheme == 'https' then
-      port.number = 443
-    else
-      port.number = 80
-    end
-  end
+  port.number = parsed.port or get_default_port(parsed.scheme)
 
   local path = parsed.path or "/"
   if parsed.query then

Please let me know if you have any questions or concerns; otherwise I will commit the patch in a few weeks.

@nnposter

This comment has been minimized.

Show comment
Hide comment
@nnposter

nnposter Apr 1, 2017

I have identified a number of scripts that could benefit from the same code abstraction.

@dmiller-nmap Do you have any concerns over moving get_default_port() to url.lua to make it consumable outside of http.lua?

nnposter commented Apr 1, 2017

I have identified a number of scripts that could benefit from the same code abstraction.

@dmiller-nmap Do you have any concerns over moving get_default_port() to url.lua to make it consumable outside of http.lua?

@nnposter

This comment has been minimized.

Show comment
Hide comment
@nnposter

nnposter Apr 1, 2017

Here is the subsequent patch that moves the function to url.lua. Seems like a nice reduction in code clutter:

--- a/nselib/data/http-default-accounts-fingerprints.lua
+++ b/nselib/data/http-default-accounts-fingerprints.lua
@@ -155,10 +155,8 @@
   local parts = tcopy(parsed or {})
   parts.host = parts.host or stdnse.get_hostname(host, port)
   parts.scheme = parts.scheme or shortport.ssl(host, port) and "https" or "http"
-  local pn = parts.port or tostring(port.number)
-  if not (parts.scheme == "http" and pn == "80"
-       or parts.scheme == "https" and pn == "443") then
-    parts.port = pn
+  if not parts.port and port.number ~= url.get_default_port(parts.scheme) then
+    parts.port = tostring(port.number)
   end
   return parts
 end
--- a/nselib/http.lua
+++ b/nselib/http.lua
@@ -158,10 +158,9 @@
 end
 
 --- Provide the default port for a given scheme.
-local function get_default_port (scheme)
-  local ports = {http=80, https=443}
-  return ports[scheme]
-end
+-- The localization is necessary because functions in http.lua like to use
+-- "url" as a local parameter
+local get_default_port = url.get_default_port
 
 --- Get a value suitable for the Host header field.
 -- See RFC 2616 sections 14.23 and 5.2.
--- a/nselib/url.lua
+++ b/nselib/url.lua
@@ -373,4 +373,16 @@
   return table.concat(qstr, '&')
 end
 
+---
+-- Provides the default port for a given URI scheme.
+--
+-- @param scheme for determining the port, such as "http" or "https".
+-- @return A port number as an integer, such as 443 for scheme "https",
+--         or nil in case of an undefined scheme
+-----------------------------------------------------------------------------
+function get_default_port (scheme)
+  local ports = {http=80, https=443}
+  return ports[(scheme or ""):lower()]
+end
+
 return _ENV;
--- a/nselib/httpspider.lua
+++ b/nselib/httpspider.lua
@@ -233,9 +233,7 @@
       base_href = base_href .. '/'
     end
 
-    if ( ( base_url:getProto() == 'https' and base_url:getPort() == 443 ) or
-        ( base_url:getProto() == 'http' and base_url:getPort() == 80 ) ) then
-
+    if base_url:getPort() == url.get_default_port(base_url:getProto()) then
       if ( leading_slash ) then
         return ("%s://%s/%s"):format(base_url:getProto(), base_url:getHost(), rel_url)
       else
@@ -427,14 +425,7 @@
     self.proto, self.host, self.port, self.file = self.raw:match("^(http[s]?)://([^:/]*)[:]?(%d*)")
     if ( self.proto and self.host ) then
       self.file = self.raw:match("^http[s]?://[^:/]*[:]?%d*(/[^#]*)") or '/'
-      self.port = tonumber(self.port)
-      if ( not(self.port) ) then
-        if ( self.proto:match("https") ) then
-          self.port = 443
-        elseif ( self.proto:match("http")) then
-          self.port = 80
-        end
-      end
+      self.port = tonumber(self.port) or url.get_default_port(self.proto)
 
       self.path  = self.file:match("^([^?]*)[%?]?")
       self.dir   = self.path:match("^(.+%/)") or "/"
--- a/scripts/http-backup-finder.nse
+++ b/scripts/http-backup-finder.nse
@@ -122,13 +122,8 @@
     if ( parsed.path:match(".*%.*.$") ) then
       -- iterate over possible backup files
       for link in backupNames(parsed.path) do
-        local host, port = parsed.host, parsed.port
-
-        -- if no port was found, try to deduce it from the scheme
-        if ( not(port) ) then
-          port = (parsed.scheme == 'https') and 443
-          port = port or ((parsed.scheme == 'http') and 80)
-        end
+        local host = parsed.host
+        local port = parsed.port or url.get_default_port(parsed.scheme)
 
         -- the url.escape doesn't work here as it encodes / to %2F
         -- which results in 400 bad request, so we simple do a space
--- a/scripts/http-favicon.nse
+++ b/scripts/http-favicon.nse
@@ -128,21 +128,17 @@
 -- host, port, and path if the URL is relative. Return nil if the scheme is not
 -- "http" or "https".
 function parse_url_relative(u, host, port, path)
-  local defaultport, scheme, abspath
+  local scheme, abspath
   u = url.parse(u)
   scheme = u.scheme or "http"
-  if scheme == "http" then
-    defaultport = 80
-  elseif scheme == "https" then
-    defaultport = 443
-  else
+  if not (scheme == "http" or scheme == "https") then
     return nil
   end
   abspath = u.path or ""
   if not string.find(abspath, "^/") then
     abspath = dirname(path) .. "/" .. abspath
   end
-  return u.host or host, u.port or defaultport, abspath
+  return u.host or host, u.port or url.get_default_port(scheme), abspath
 end
 
 function parseIcon( body )
--- a/scripts/http-open-redirect.nse
+++ b/scripts/http-open-redirect.nse
@@ -54,13 +54,7 @@
 end
 
 local function getHostPort(parsed)
-  local host, port = parsed.host, parsed.port
-  -- if no port was found, try to deduce it from the scheme
-  if ( not(port) ) then
-    port = (parsed.scheme == 'https') and 443
-    port = port or ((parsed.scheme == 'http') and 80)
-  end
-  return host, port
+  return parsed.host, parsed.port or url.get_default_port(parsed.scheme)
 end
 
 local function isRedirect(status)
--- a/scripts/http-phpself-xss.nse
+++ b/scripts/http-phpself-xss.nse
@@ -143,12 +143,8 @@
 
     --Only work with .php files
     if ( parsed.path and parsed.path:match(".*.php") ) then
-        --The following port/scheme code was seen in http-backup-finder and its neat =)
-        local host, port = parsed.host, parsed.port
-        if ( not(port) ) then
-          port = (parsed.scheme == 'https') and 443
-          port = port or ((parsed.scheme == 'http') and 80)
-        end
+        local host = parsed.host
+        local port = parsed.port or url.get_default_port(parsed.scheme)
         local escaped_link = parsed.path:gsub(" ", "%%20")
         if launch_probe(host,port,escaped_link) then
           table.insert(vulnpages, parsed.scheme..'://'..host..escaped_link..PHP_SELF_PROBE)
--- a/scripts/http-unsafe-output-escaping.nse
+++ b/scripts/http-unsafe-output-escaping.nse
@@ -49,14 +49,9 @@
 local dbg = stdnse.debug2
 
 local function getHostPort(parsed)
-  local host, port = parsed.host, parsed.port
-  -- if no port was found, try to deduce it from the scheme
-  if ( not(port) ) then
-    port = (parsed.scheme == 'https') and 443
-    port = port or ((parsed.scheme == 'http') and 80)
-  end
-  return host, port
+  return parsed.host, parsed.port or url.get_default_port(parsed.scheme)
 end
+
 local function getReflected(parsed, r)
   local reflected_values,not_reflected_values = {},{}
   local count = 0

nnposter commented Apr 1, 2017

Here is the subsequent patch that moves the function to url.lua. Seems like a nice reduction in code clutter:

--- a/nselib/data/http-default-accounts-fingerprints.lua
+++ b/nselib/data/http-default-accounts-fingerprints.lua
@@ -155,10 +155,8 @@
   local parts = tcopy(parsed or {})
   parts.host = parts.host or stdnse.get_hostname(host, port)
   parts.scheme = parts.scheme or shortport.ssl(host, port) and "https" or "http"
-  local pn = parts.port or tostring(port.number)
-  if not (parts.scheme == "http" and pn == "80"
-       or parts.scheme == "https" and pn == "443") then
-    parts.port = pn
+  if not parts.port and port.number ~= url.get_default_port(parts.scheme) then
+    parts.port = tostring(port.number)
   end
   return parts
 end
--- a/nselib/http.lua
+++ b/nselib/http.lua
@@ -158,10 +158,9 @@
 end
 
 --- Provide the default port for a given scheme.
-local function get_default_port (scheme)
-  local ports = {http=80, https=443}
-  return ports[scheme]
-end
+-- The localization is necessary because functions in http.lua like to use
+-- "url" as a local parameter
+local get_default_port = url.get_default_port
 
 --- Get a value suitable for the Host header field.
 -- See RFC 2616 sections 14.23 and 5.2.
--- a/nselib/url.lua
+++ b/nselib/url.lua
@@ -373,4 +373,16 @@
   return table.concat(qstr, '&')
 end
 
+---
+-- Provides the default port for a given URI scheme.
+--
+-- @param scheme for determining the port, such as "http" or "https".
+-- @return A port number as an integer, such as 443 for scheme "https",
+--         or nil in case of an undefined scheme
+-----------------------------------------------------------------------------
+function get_default_port (scheme)
+  local ports = {http=80, https=443}
+  return ports[(scheme or ""):lower()]
+end
+
 return _ENV;
--- a/nselib/httpspider.lua
+++ b/nselib/httpspider.lua
@@ -233,9 +233,7 @@
       base_href = base_href .. '/'
     end
 
-    if ( ( base_url:getProto() == 'https' and base_url:getPort() == 443 ) or
-        ( base_url:getProto() == 'http' and base_url:getPort() == 80 ) ) then
-
+    if base_url:getPort() == url.get_default_port(base_url:getProto()) then
       if ( leading_slash ) then
         return ("%s://%s/%s"):format(base_url:getProto(), base_url:getHost(), rel_url)
       else
@@ -427,14 +425,7 @@
     self.proto, self.host, self.port, self.file = self.raw:match("^(http[s]?)://([^:/]*)[:]?(%d*)")
     if ( self.proto and self.host ) then
       self.file = self.raw:match("^http[s]?://[^:/]*[:]?%d*(/[^#]*)") or '/'
-      self.port = tonumber(self.port)
-      if ( not(self.port) ) then
-        if ( self.proto:match("https") ) then
-          self.port = 443
-        elseif ( self.proto:match("http")) then
-          self.port = 80
-        end
-      end
+      self.port = tonumber(self.port) or url.get_default_port(self.proto)
 
       self.path  = self.file:match("^([^?]*)[%?]?")
       self.dir   = self.path:match("^(.+%/)") or "/"
--- a/scripts/http-backup-finder.nse
+++ b/scripts/http-backup-finder.nse
@@ -122,13 +122,8 @@
     if ( parsed.path:match(".*%.*.$") ) then
       -- iterate over possible backup files
       for link in backupNames(parsed.path) do
-        local host, port = parsed.host, parsed.port
-
-        -- if no port was found, try to deduce it from the scheme
-        if ( not(port) ) then
-          port = (parsed.scheme == 'https') and 443
-          port = port or ((parsed.scheme == 'http') and 80)
-        end
+        local host = parsed.host
+        local port = parsed.port or url.get_default_port(parsed.scheme)
 
         -- the url.escape doesn't work here as it encodes / to %2F
         -- which results in 400 bad request, so we simple do a space
--- a/scripts/http-favicon.nse
+++ b/scripts/http-favicon.nse
@@ -128,21 +128,17 @@
 -- host, port, and path if the URL is relative. Return nil if the scheme is not
 -- "http" or "https".
 function parse_url_relative(u, host, port, path)
-  local defaultport, scheme, abspath
+  local scheme, abspath
   u = url.parse(u)
   scheme = u.scheme or "http"
-  if scheme == "http" then
-    defaultport = 80
-  elseif scheme == "https" then
-    defaultport = 443
-  else
+  if not (scheme == "http" or scheme == "https") then
     return nil
   end
   abspath = u.path or ""
   if not string.find(abspath, "^/") then
     abspath = dirname(path) .. "/" .. abspath
   end
-  return u.host or host, u.port or defaultport, abspath
+  return u.host or host, u.port or url.get_default_port(scheme), abspath
 end
 
 function parseIcon( body )
--- a/scripts/http-open-redirect.nse
+++ b/scripts/http-open-redirect.nse
@@ -54,13 +54,7 @@
 end
 
 local function getHostPort(parsed)
-  local host, port = parsed.host, parsed.port
-  -- if no port was found, try to deduce it from the scheme
-  if ( not(port) ) then
-    port = (parsed.scheme == 'https') and 443
-    port = port or ((parsed.scheme == 'http') and 80)
-  end
-  return host, port
+  return parsed.host, parsed.port or url.get_default_port(parsed.scheme)
 end
 
 local function isRedirect(status)
--- a/scripts/http-phpself-xss.nse
+++ b/scripts/http-phpself-xss.nse
@@ -143,12 +143,8 @@
 
     --Only work with .php files
     if ( parsed.path and parsed.path:match(".*.php") ) then
-        --The following port/scheme code was seen in http-backup-finder and its neat =)
-        local host, port = parsed.host, parsed.port
-        if ( not(port) ) then
-          port = (parsed.scheme == 'https') and 443
-          port = port or ((parsed.scheme == 'http') and 80)
-        end
+        local host = parsed.host
+        local port = parsed.port or url.get_default_port(parsed.scheme)
         local escaped_link = parsed.path:gsub(" ", "%%20")
         if launch_probe(host,port,escaped_link) then
           table.insert(vulnpages, parsed.scheme..'://'..host..escaped_link..PHP_SELF_PROBE)
--- a/scripts/http-unsafe-output-escaping.nse
+++ b/scripts/http-unsafe-output-escaping.nse
@@ -49,14 +49,9 @@
 local dbg = stdnse.debug2
 
 local function getHostPort(parsed)
-  local host, port = parsed.host, parsed.port
-  -- if no port was found, try to deduce it from the scheme
-  if ( not(port) ) then
-    port = (parsed.scheme == 'https') and 443
-    port = port or ((parsed.scheme == 'http') and 80)
-  end
-  return host, port
+  return parsed.host, parsed.port or url.get_default_port(parsed.scheme)
 end
+
 local function getReflected(parsed, r)
   local reflected_values,not_reflected_values = {},{}
   local count = 0

@nmap-bot nmap-bot closed this in e80976a Apr 19, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment