[Snort-devel] RE: Snort 1.9.1 fix for stream4 reassembly integer overflow

Dave Greenstein dave at ...1918...
Tue Apr 15 20:00:02 EDT 2003


 
One other note, to reproduce the vulnerability using the hping method
described by CORE security, make sure you wait for the first two hping
commands to complete before returning with the last hping send. They
take a while given the size of the stream being sent.
 
Just noticed Matt Callaway had a simpler fix for the 1.9.1 code base...
not sure which is preferable.
 
Dave

	-----Original Message-----
	From: Dave Greenstein 
	Sent: Tuesday, April 15, 2003 8:43 PM
	To: 'snort-devel at lists.sourceforge.net'
	Subject: Snort 1.9.1 fix for stream4 reassembly integer overflow
	
	
	 
	I'm working on a fix to the snort 1.9.1 code base for the Snort
TCP stream reassembly integer overflow vulnerability. Basically, I've
merged the fix from the snort 2.0.0 RC1 changes into the 1.9.1 code.
I've noticed some discrepencies in the logic of
spp_stream4.c:Traversefunc() that I've commented in the code below. I'm
not sure if these logic changes are intentional or if they've introduced
new bugs. Here is a list of the changes I've made:
	 
	1. added the bounds.c file to 1.9.1.
	2. added the isBetween inline function to spp_stream4.c
	3. changed the TraverseFunc function to use the new logic from
snort 2.0.0.
	4. added a few other new defines to decode.h and spp_stream4.h.
	 
	I verified that the vulnerability is fixed, but, I'm not sure if
the discrepencies, listed below (search for "NOT THE SAME"), from the
2.0.0 logic and original 1.9.1 logic have introduced new problems.
Anyone have any insight into this? I'll submit the entire change once
I've verified the code is good. Thanks!
	 
	Dave
	 
	 
	 
	static INLINE int isBetween(u_int32_t low, u_int32_t high,
u_int32_t cur)
	{
	        return (cur - low) <= (high - low);
	}
	 
	... ...
	 
	static void TraverseFunc(ubi_trNodePtr NodePtr, void
*build_data)
	{
	    Stream *s;
	    StreamPacketData *spd;
	    BuildData *bd;
	    u_int8_t *buf;
	    int trunc_size;
	    int offset = 0;
	 
	    if(s4data.stop_traverse)
	        return;
	 
	    spd = (StreamPacketData *) NodePtr;
	    bd = (BuildData *) build_data;
	    s = bd->stream;
	    buf = bd->buf;
	 
	/* don't reassemble if we're before the start sequence number or

	* after the last ack'd byte
	*/
	    if(!isBetween(s->base_seq, s->last_ack, spd->seq_num)) {
	        // if(spd->seq_num < s->base_seq || spd->seq_num >
s->last_ack) {
	        DEBUG_WRAP(DebugMessage(DEBUG_STREAM,
	                            "not reassembling because"
	                            " we're (%u) before isn(%u) or after
last_ack(%u)\n",
	        spd->seq_num, s->base_seq, s->last_ack););
	        return;
	    }
	 
	    /* if it's in bounds... */
	    if(isBetween(s->base_seq, s->last_ack, spd->seq_num) &&
	      isBetween(s->base_seq, s->last_ack,
(spd->seq_num+spd->payload_size)))
	            // if(spd->seq_num >= s->base_seq && spd->seq_num >=
s->next_seq &&
	            // (spd->seq_num+spd->payload_size) <= s->last_ack)
	            // NOT THE SAME: old comparisons have s->next_seq
	    {
	        offset = spd->seq_num - s->base_seq;
	 
	        s->next_seq = spd->seq_num + spd->payload_size;
	        DEBUG_WRAP(DebugMessage(DEBUG_STREAM, "Copying %d bytes
into buffer, "
	                                "offset %d, buf %p\n",
spd->payload_size, offset, 
	                                buf););
	        DEBUG_WRAP(DebugMessage(DEBUG_STREAM,
	                                "spd->seq_num (%u) s->last_ack
(%u) s->base_seq(%u) size: (%u) s->next_seq(%u), offset(%u)\n",
	                                spd->seq_num, s->last_ack,
s->base_seq,
	                                spd->payload_size, s->next_seq,
offset));
	 
	        SafeMemcpy(buf+offset, spd->payload, spd->payload_size,
	        stream_pkt->data, stream_pkt->data + MAX_STREAM_SIZE);
	        // memcpy(buf+offset, spd->payload, spd->payload_size);
	        pc.rebuilt_segs++;
	        spd->chuck = 1;
	        bd->total_size += spd->payload_size;
	    } 
	    else if(isBetween(s->base_seq, s->last_ack, spd->seq_num) &&
	            ((spd->seq_num + spd->payload_size) > s->last_ack))
	            // else if(spd->seq_num >= s->base_seq && 
	            // spd->seq_num < s->last_ack &&
	            // spd->seq_num + spd->payload_size > s->last_ack)
	            // NOT THE SAME: OLD upper bound exclusive,
	            // NEW upper bound inclusive
	    {
	        /*
	        * if it starts in bounds and hasn't been completely
ack'd, 
	        * truncate the last piece and copy it in 
	        */
	        trunc_size = s->last_ack - spd->seq_num; 
	        DEBUG_WRAP(DebugMessage(DEBUG_STREAM, "Truncating
overlap of %d bytes\n", 
	                                spd->seq_num + spd->payload_size
- s->last_ack);
	                                DebugMessage(DEBUG_STREAM, " =>
trunc info seq: 0x%X "
	                                "size: %d last_ack: 0x%X\n", 
	                                spd->seq_num, spd->payload_size,
s->last_ack);
	                                );
	        offset = spd->seq_num - s->base_seq;
	        if(trunc_size < (65500-offset) && trunc_size > 0)
	        {
	            DEBUG_WRAP(DebugMessage(DEBUG_STREAM, "Copying %d
bytes into buffer, "
	                                    "offset %d, buf %p\n",
trunc_size, offset, 
	                                    buf););
	            SafeMemcpy(buf+offset, spd->payload, trunc_size,
	            stream_pkt->data, stream_pkt->data +
MAX_STREAM_SIZE); 
	            // memcpy(buf+offset, spd->payload, trunc_size);
	            pc.rebuilt_segs++;
	            bd->total_size += trunc_size;
	       }
	       else
	        {
	            DEBUG_WRAP(DebugMessage(DEBUG_STREAM, "Woah, got bad
TCP segment "
	                                    "trunctation value (%d)\n",
trunc_size););
	        }
	 
	        spd->chuck = 1;
	    }
	    else if(spd->seq_num < s->base_seq && 
	            isBetween(s->base_seq, s->last_ack,
(spd->seq_num+spd->payload_size)))
	                // else if(spd->seq_num < s->base_seq && 
	                // spd->seq_num+spd->payload_size > s->base_seq)
	                // NOT THE SAME: else if(spd->seq_num <
s->base_seq && s_base_seq < spd->seq_num+spd->payload_size)
	    {
	        /* case where we've got a segment that wasn't completely
ack'd 
	        * last time it was processed, do a partial copy into the
buffer
	        */
	        DEBUG_WRAP(DebugMessage(DEBUG_STREAM, "Incompleted
segment, copying up "
	                                "to last-ack\n"););
	        /* calculate how much un-ack'd data to copy */
	        trunc_size = (spd->seq_num+spd->payload_size) -
s->base_seq;
	        /* figure out where in the original data payload to
start copying */
	        offset = s->base_seq - spd->seq_num;
	 
	        if(trunc_size < 65500 && trunc_size > 0)
	        {
	            DEBUG_WRAP(DebugMessage(DEBUG_STREAM, "Copying %d
bytes into buffer, "
	                                    "offset %d, buf %p\n",
trunc_size, offset, 
	                                    buf););
	            SafeMemcpy(buf, spd->payload+offset, trunc_size,
	            stream_pkt->data, stream_pkt->data +
MAX_STREAM_SIZE); 
	            // memcpy(buf, spd->payload+offset, trunc_size);
	            pc.rebuilt_segs++;
	            bd->total_size += trunc_size;
	        }
	        else
	        {
	            DEBUG_WRAP(DebugMessage(DEBUG_STREAM, "Woah, got bad
TCP segment "
	            "trunctation value (%d)\n", trunc_size););
	        }
	    }
	    else if(spd->seq_num < s->base_seq)
	    {
	        /* ignore this segment, we've already looked at it */
	        return;
	    }
	    else if(spd->seq_num > s->last_ack)
	    {
	        /* we're all done, we've walked past the end of the
ACK'd data */
	        DEBUG_WRAP(DebugMessage(DEBUG_STREAM, " => Segment is
past last ack'd data, "
	                                "ignoring for now...\n");
	        DebugMessage(DEBUG_STREAM, " => (%d bytes @ seq 0x%X, "
	                                "ack: 0x%X)\n",
spd->payload_size, spd->seq_num, s->last_ack);
	                                );
	        /* since we're reassembling in order, once we hit an
overflow condition
	        * let's stop trying for now
	        */
	        s4data.stop_traverse = 1;
	        s4data.stop_seq = spd->seq_num;
	    }
	    else if(spd->seq_num < s->next_seq)
	    {
	        if(s4data.evasion_alerts)
	        {
	            Event event;
	            Packet p;
	            bzero(&p, sizeof(Packet));
	            /* go through and generate enough of a packet to
generate an alert off of */
	            (*grinder)(&p, (struct pcap_pkthdr *) &spd->pkth,
spd->pkt);
	            SetEvent(&event, GENERATOR_SPP_STREAM4, 
	            STREAM4_MULTIPLE_ACKED, 1, 0, 5, 0);
	            CallAlertFuncs(&p, STREAM4_MULTIPLE_ACKED_STR, NULL,
&event);
	            CallLogFuncs(&p, STREAM4_MULTIPLE_ACKED_STR, NULL,
&event);
	        }
	        DEBUG_WRAP(DebugMessage(DEBUG_STREAM,
	                                "The seq_num is less than it
should have been\n"););
	        return;
	    }
	    else
	    {
	        DEBUG_WRAP(DebugMessage(DEBUG_STREAM,
	                                "Ended up in the default case
somehow.. !\n"
	                                "spd->seq_num(%u)
s->next_seq(%u)\n"
	                    ););
	 
	    }
	 
	} 
	 

	. . .
	David Greenstein
	Senior Engineer
	Latis Networks, Inc.
	
	303-642-4508 Direct
	303-642-4501 Fax
	
	www.stillsecure.com <http://www.stillsecure.com/> 
	Reducing your risk has never been this easy.
	. . .
	The information transmitted is intended only for the person
	to which it is addressed and may contain confidential material.
	Review or other use of this information by persons other than
	the intended recipient is prohibited. If you've received
	this in error, please contact the sender and delete
	from any computer. 

	 

	 

	. . .
	David Greenstein
	Senior Engineer
	Latis Networks, Inc.
	
	303-642-4508 Direct
	303-642-4501 Fax
	
	www.stillsecure.com <http://www.stillsecure.com/> 
	Reducing your risk has never been this easy.
	. . .
	The information transmitted is intended only for the person
	to which it is addressed and may contain confidential material.
	Review or other use of this information by persons other than
	the intended recipient is prohibited. If you've received
	this in error, please contact the sender and delete
	from any computer. 

	 

	 

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.snort.org/pipermail/snort-devel/attachments/20030415/b4ec74a0/attachment.html>


More information about the Snort-devel mailing list