Nmap Development mailing list archives

Re: [NSE] http.lua and delimiters


From: Sven Klemm <sven () c3d2 de>
Date: Wed, 01 Oct 2008 15:42:08 +0200

Sven Klemm wrote:
David Fifield wrote:
I think using a heuristic to get the header delimiter is fine. Wget does
it: it splits the header from the body by looking for \n\n or \n\r\n,
and splits header lines on either \n or \r\n. cURL does it: it ends
header lines on \n and and the header on a line beginning with \r or \n.
The only thing I would do differently is this bit of code:

  -- try and separate the head from the body
  if response:match( "\r\n\r\n" ) then
    header, body = response:match( "^(.-)\r\n\r\n(.*)$" )
  elseif response:match( "\n\n" ) then
    header, body = response:match( "^(.-)\n\n(.*)$" )

This would fail if the header uses \n delimiters but there is an
\r\n\r\n somewhere in the body; the first match would succeed and grab
part of the body with the header. What you want is whichever of those
two matches gives you a shorter header.

Guessing the line ending for the header is fine, but we shouldn't do
anything like that for the body. We don't even know that the body is
made up of "lines"; it should be treated as a block of data, in other
words not split up and rejoined. In the case of chunked encoding, cURL
is strict about requiring \r\n, so we should be safe doing the same.
Wget doesn't do chunked.

Maybe you can modify your script to report servers that send chunked
encoding but delimit the chunks with bare \n? It's hard to imagine any
kind of reliable relivery of chunked encoding if you have to guess at
the line endings.

Does anyone else have more experience dealing with HTTP? My feeling is
that there should be no guessing of delimiters in the body. The
technique of splitting on \r\n for the purposes of dechunking is fine,
because that's at a higher layer, above the raw body.


I've attached a patch which adds those features to the http library a little differently than Jah's patch. The patch changes the http library to use the comm library which leads to less code. Using the comm library has one disadvantage though: We cannot specify different timeouts for connect and receive as the http library does currently. I think it's not necessary to specify different values for those. But I left those different values in for now. Either we remove them or we don't use comm.lua for receiving and add our own receive loop or we add this feature to comm.lua.
The patch makes use of the eof option for the comm library.

The version sent removed "\r\n" from the body when using chunked encoding the attached version fixes this problem. Kris already sent a patch for comm.lua to add support for different timeouts for connect and receive but it hasn't been applied yet. So using comm.lua doesn't have any disadvantages.

Cheers,
Sven

--
Sven Klemm
http://cthulhu.c3d2.de/~sven/

Index: nselib/http.lua
===================================================================
--- nselib/http.lua     (revision 10425)
+++ nselib/http.lua     (working copy)
@@ -16,6 +16,7 @@
 
 require 'stdnse'
 require 'url'
+require 'comm'
 
 --
 -- http.get( host, port, path, options )
@@ -125,36 +126,26 @@
   end
 
   local result = {status=nil,header={},body=""}
-  local socket = nmap.new_socket()
-  local default_timeout = {}
-  if options.timeout then
-    socket:set_timeout( options.timeout )
-  else
-    default_timeout = get_default_timeout( nmap.timing_level() )
-    socket:set_timeout( default_timeout.connect )
-  end
-  if not socket:connect( host, port, protocol ) then
-    return result
-  end
-  if not options.timeout then
-    socket:set_timeout( default_timeout.request )
-  end
-  if not socket:send( data ) then
-    return result
-  end
+  local timeout, status, response
 
-  local buffer = stdnse.make_buffer( socket, "\r\n" )
+  timeout = options.timeout or get_default_timeout( nmap.timing_level() ).connect
 
-  local line, _
-  local header, body = {}, {}
+  status, response = comm.exchange( host, port, data, {proto=protocol,timeout=timeout,eof=true})
+  
+  if not status then
+    return result
+  end
 
-  -- header loop
-  while true do
-    line = buffer()
-    if (not line or line == "") then break end
-    table.insert(header,line)
+  local header, body
+  if response:match( "\r?\n\r?\n" ) then
+    header, body = response:match( "^(.-)\r?\n\r?\n(.*)$" )
+  else
+    -- if we can't separate header and body return everything as body
+    header, body = "", response
   end
 
+  header = stdnse.strsplit( "\r?\n", header )
+
   -- build nicer table for header
   local last_header, match
   for number, line in ipairs( header ) do
@@ -187,32 +178,24 @@
   -- handle body
   if result.header['transfer-encoding'] == 'chunked' then
     -- if the server used chunked encoding we have to 'dechunk' the answer
-    local counter, chunk_size
-    counter = 0; chunk_size = 0
-    while true do
+    body_chunks = {}
+    local counter, chunk_size = 0, 0
+    for _, line in pairs( stdnse.strsplit( "\r\n", body ) ) do
       if counter >= chunk_size then
         counter = 0
-        chunk_size = tonumber( buffer(), 16 )
+        chunk_size = tonumber( line, 16 )
         if chunk_size == 0 or not chunk_size then break end
+      else
+        counter = counter + #line + 2
+        table.insert( body_chunks, line) 
       end
-      line = buffer()
-      if not line then break end
-      counter = counter + #line + 2
-      table.insert(body,line)
     end
+    result.body = table.concat( body_chunks, "\r\n" )
   else
-    while true do
-      line = buffer()
-      if not line then break end
-      table.insert(body,line)
-    end
+    result.body = body
   end
 
-  socket:close()
-  result.body = table.concat( body, "\r\n" )
-
   return result
-
 end
 
 get_default_timeout = function( nmap_timing )

_______________________________________________
Sent through the nmap-dev mailing list
http://cgi.insecure.org/mailman/listinfo/nmap-dev
Archived at http://SecLists.Org

Current thread: