[Snort-devel] Yet more data corruption in stream4/snort-2.0.5
jeff at ...835...
Wed Dec 3 13:05:00 EST 2003
-----BEGIN PGP SIGNED MESSAGE-----
On Dec 2, 2003, at 5:22 PM, Andrew Rucker Jones wrote:
> -----BEGIN PGP SIGNED MESSAGE-----
> Hash: SHA1
> Hi All!
> There is another data corruption ... deficiency ... in stream4 in
> 2.0.5. This one stems from the fact that snort does not handle TCP
> window scaling. If (nonzero) window scales are in use, and one side of
> the connection pushes more data (without an ACK) to the other side than
> the 16 bit window size field in the TCP header allows, but not more
> the window header << window scale (which is why the TCP window scaling
> exists to begin with), snort will complain of a window violation error,
> and will ignore the packet. Later, of course, when it goes to rebuild
> the stream, that packet is missing, and whatever was in the rebuild
> buffer from the last flush remains.
Thanks for taking the time to look at this. We've spoken (the dev
team) about supporting window scaling and other options such as
selective acknowledgment but our position has been that they're
sufficiently complex to require a lot of thought before we took a stab
at how to properly deal with them.
> The attached patches add window scaling support to snort. There is a
> little something more to say about them:
> 1) The patch for decode.h also changes the type of the len field in the
> Options structure from u_int32_t to u_int8_t. RFCs 760 and 761 state
> that the length of any IP or TCP options must fit in one byte, so i
> don't understand why this was ever a 32 bit value.
This is probably a strange artifact. I'm not sure what the effect will
be without looking at it more closely. You are right insofar as a TCP
option length can be specified in a u_int8_t. The reason behind using
a u_int32_t might be more for alignment of the super structure than
> 2) I wasn't sure if alerts were desired if funky TCP options are
> detected, or if the window scale is greater than the maximum allowed
> value of 14, so i didn't put them in. (I don't really know how to,
The might be... it's worth looking into.
> 3) One data corruption problem remains for sure, which i mentioned in
> the comments in the spp_stream4.c patch: if snort picks up a session in
> the middle, and nonzero window scales are in use, snort will not know
> that, and data corruption is certain to occur if large amounts of data
> are being transfered. As i also say in the comments, this is kind of
That is clearly one problem. One of the most significant problems with
supporting window scaling in the first place is that it provides an
attacker many opportunities for insertion and other types of attacks.
It makes knowing when to flush more difficult and it may make memory
management more difficult as we have to hang on to more of the stream
before flushing it.
> Now i have some questions:
> 1) What is with WithinSessionLimits() in spp_stream4.c? It always
> returns 1!
I'll get back to you on that (Or, rather.. I suspect Chris Green will).
> 2) Has anyone ever seen a case where one side of a connection
> acknowledges data that have not yet been sent? I swear that i have a
> real world capture (an e-mail to a Yahoo! address) in which the client
> sends data up to sequence number X, after which the server sends two
> ACKs right after each other: one for X, and one for X+1024 or so. Right
> after that, the client sends the next 1024 bytes. There are no
> retransmits involved. Of course, that causes data corruption, too, but
> snort can hardly be blamed for that. That entire conversation was just
> as weird as can be, i have to say.
This might be the result of an "optimization" algorithm, ACKs arriving
out of sequence or any number of other issues.
Do you have a capture we could look at?
> - --
> GPG key / Schlüssel -- http://simultan.dyndns.org/~arjones/gpgkey.txt
> Encrypt everything. / Alles verschlüsseln.
The original EZ-bake packet oven.
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.3 (Darwin)
-----END PGP SIGNATURE-----
More information about the Snort-devel