Nmap Development: Re: [NSE] http.lua and delimiters

[Sven Klemm <sven () c3d2 de>]

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.

--
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 )
   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