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

Jason V. Miller jmiller at ...417...
Thu Apr 24 06:48:13 EDT 2003


Chris,

Thanks for your quick response. What's the word on stuff like this getting fixed
in Snort 2.0 with some sort of interim release? Is byte_test (etc) officially
supported, or is it some sort of beta keyword?

Also, going from memory, I'm pretty sure that the same bug exists in the code
for 4-byte conversions. Hrm. Actually, looking at the code (unless I've screwed
my local copy), it looks to be as though the bug for 4-byte conversions is
present for big endian systems only.

J.

On Thu, Apr 24, 2003 at 09:28:51AM -0400, Chris Green wrote:
> "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;)
> >
> 
B
> [...]
> 
> > 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.

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




More information about the Snort-devel mailing list