Re: [NSE] socket:receive_buf(...) broken

[David Fifield <david () bamsoftware com>]

On Tue, Sep 14, 2010 at 02:24:27AM -0400, Patrick Donnelly wrote:
> I'm in process of a major maintenance fix of the NSE nsock binding. In
> the process of doing so I have found numerous bugs and problems but
> one has been extra glaring that warrants a post here, now.
>
> The socket:receive_buf [1] function has numerous problems:
>
> When *any* socket method is called, excluding receive_buf, the buffer
> is cleared (especially send!) [1]. This is "intentional behavior" but
> has resulted in buggy usage of the function. For example, the ssh2.lua
> library throws away the last 4 bytes of a payload because it does not
> offset the initial integer giving the length. These extra four bytes
> are kept in the buffer but discarded on a later send. These are good
> bytes that were part of the payload! I assume these bytes were some
> form of padding and that is the only reason any ssh operation
> succeeds.
>
> These are the design problems I'm so far aware of with this function.
> I'm fixing all of the above in the new implementation. To rehash the
> changes to receive_buf:
>
> o The buffer will no longer be cleared on a call to other socket
> methods. Only receive_buf reads the buffer. Other read methods will
> ignore the contents of the buffer. Do not interleave calls without
> finishing the buffer.
> o A "priming" call to the pattern function may pass a buffer of length
> 0. The function/pattern should handle this appropriately. (This needs
> to be the case anyway; consider in the above example if for some
> reason we only had 2 bytes in the buffer. The read of a 4 byte integer
> would still fail!)
> o receive_buf does not automatically set keep to true if the two
> integers returned by the pattern function are equal.

Good job finding this and good analysis. I've been watching your
-maintenance commits and I think that the solution looks good.

I would like to eventually remove receive_buf completely. I believe that
such a function should be built into Nsock itself. There should be a
buffer associated with each iod so that you can ask for a certain number
of bytes (or lines, etc.) and get exactly that many, no more, with those
left over remaining in the buffer. The ssh pattern would be

packet_size = socket:receive_exactly(4)
-- Unpack size.
packet = socket:receive_exactly(packet_size)

It would be an error if either call failed; they would return an error
if they can't supply enough bytes. There would also be a call to return
everything available on the socket, clearing the buffer. There is such a
socket buffer (not using Nsock) in ncat/http.c.

The main implementation difficulty that I have considered is that you
can't just use the select function to detect when an iod has input
pending, you also have to check if there is anything in the buffer. We
would also need new name for the new "exact" functions. There are a lot
of script that use receive_bytes(1) that expect to get as many bytes as
are available.

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