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

Jason V. Miller jmiller at ...417...
Wed Apr 23 21:59:07 EDT 2003


Hey everyone,

First of all, I'd just like to say that I'm really happy with the introduction
of the byte_test keyword into Snort. From the perspective of anyone who writes
signatures on a semi-regular basis, it's a life saver. On to the point. I've
found what seems to be a major bug in the ByteTest() function (and as such the
byte_test rule keyword) on all little endian systems. While developing a
signature for a vulnerability in a PPTP daemon, I created a rule similar to the
following:

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

The rule is pretty straightforward, and is simply designed to pick up any PPTP
control data with a length value in the PPTP header (2 bytes at offset 0) of 11
or less. After testing the rule out, I was surprised to see that it didn't
trigger on an exploitation attempt in a lab environment. After several attempts
at getting the rule to trigger, and an attempt to pinpoint the issue with the
rule, I found my way into the ByteTest() function in sp_byte_check.c (line 367).
Inside this function, you'll find the following code snippet (which only
includes the code for 2-byte conversions for brevity):

            case 2:
#if BYTE_ORDER == BIG_ENDIAN
                /* Native byte order on my native arch */
                if(btd->endianess == BIG)
                {
                    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;
                }
#else /* BYTE_ORDER == LITTLE_ENDIAN */
                /* Native byte order on my native arch */
                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;
                }
#endif /* BYTE_ORDER */

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. 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? 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). So, what gives? Am I missing something
totally obvious, or is this a legitimate bug? I ran this past a team member, and
he validated my sanity, and also informed me that lots of the Snort developers
work on big endian systems (in which case the bug isn't present), which would
put some sense to the fact that, if this is indeed a bug, that it was missed
during testing. Oh, one more note; the code snippets in question appear in both
Snort 2.0 and CVS current.

Someone care to enlighten me?

J.

-- 
Jason V. Miller, Threat Analyst
Symantec, Inc. - www.symantec.com
E-Mail:	jmiller at ...417...




More information about the Snort-devel mailing list