[Snort-devel] Bug in ByteTest() function / byte_test keyword on little endian systems?

Chris Green cmg at ...402...
Thu Apr 24 06:30:17 EDT 2003


"Jason V. Miller" <jmiller at ...417...> writes:
>
> alert tcp $EXTERNAL_NET any -> $HOME_NET 1723 \
> (msg: "PPTP control packet with small length value"; \
> flow:to_server,established; \
> byte_test: 2,<,12,0; \
> content: "|1A2B3C4D|"; offset: 4; depth:4; \
> reference:bugtraq,7316; rev: 1;)
>

[...]

> I have a few questions about this. 
>
> Firstly, I was confused to see endian-specific code in this section,
> as I'm not really sure where it would come into play with these
> sorts of operations

I believe it comes from the | use for conversion and requiring
knowledge of how the host stores it's ints. See comments below.

> As the packet data is stored in memory as an array of unsigned chars
> (or equivalent), endianess shouldn't play a part in how the data is
> retrieved from the array (we define how the data should be
> interpreted with our byte_test keyword).  Now, not being familiar
> with too many architectures, I might be missing something, but this
> is besides the point.  Looking at the code for 2-byte data
> conversion on little endian machines, we find the following:
>
>                if(btd->endianess == LITTLE)
>                 {
>                     value = (((u_int8_t) *(base_ptr)) & 0xFF) << 8;
>                     value |= (((u_int8_t) *(base_ptr+1)) & 0xFF);
>                 }
>                 else
>                 {
>                     /* Please swap me mr. byteorder */
>                     value = (((u_int8_t) *(base_ptr)) & 0xFF);
>                     value |= (((u_int8_t) *(base_ptr+1)) & 0xFF) << 8;
>                 }
>
> Is it just me, or is this backwards? 


Yes I will agree that's a bug, stemming from almost all the test cases
we'd performed were 4 byte values.  The comment about most of us using
big endian machines was accurate for when this was developed.
However, everyone but Marty will be using little endian machines after
this week.


> Both little endian and big endian machines should be able to perform
> this 2-byte data conversion in an endian-agnostic way:
>
> 	if(btd->endianess == LITTLE) {
> 		value = (((u_int8_t) *(base_ptr)) & 0xFF);
> 		value += (((u_int8_t) *(base_ptr+1)) & 0xFF) << 8;
> 	} else {
> 		value = (((u_int8_t) *(base_ptr)) & 0xFF) << 8;
> 		value += (((u_int8_t) *(base_ptr+1)) & 0xFF);
> 	}
>
> Mind the +='s. I personally find them much easier to read than |= in this case
> (thinking about OR operations between single-byte-casted values and 4-byte
> values always makes my head spin).

Yes that is much easier to read. Thanks for the detailed
analysis. I'll spend some time converting that and making byte_jump
and byte_test use the same code path for this operation ( something
that was already on my todo list )

Thanks,
Chris
-- 
Chris Green <cmg at ...402...>
A watched process never cores.




More information about the Snort-devel mailing list