Pantek Library
Hosting Provided By
CybrHost
High Speed Hosting
Mailing Lists: snort.org > snort-devel > 03 > 040380.html (Request Expert snort.org Support)

[Snort-devel] snort-1.9.1: patch for stream4 and frag2

From: Matthew Callaway <matt-snort(at)securepipe.com>
Date: Tue Apr 29 2003 - 18:45:53 EDT


Several days ago, I posted a patch to the stream4 preprocessor that addressed the integer overflow identified by the Core team. This is a final revision of that patch that also fixes frag2.

In addition to the application of the isBetween() and SafeMemcpy() functions, I also check the return code of SafeMemcpy() to see if we skip the memcpy() due to the integer overflow. We now log an error message when we return a corrupted reassembled packet, instead of simply returning silently.

Chris Green again encourages us to make the move to 2.0, but for those of us who are, for the moment, stuck with 1.9.1, this should help.

Matt

P.S. You could change ErrorMessage to LogMessage to send the warning to syslog. 

o-------------------------------o
| Matthew Callaway              |
| Project Manager             o-------------------------o
| Firewall and VPN Technology | matt@securepipe.com     |
| SecurePipe, Inc.            | Tel: 608.294.6940       |
o-----------------------------| Fax: 608.294.6950       |
                              | Web: www.securepipe.com |
                              o-------------------------o


diff -ruN snort-1.9.1.orig/src/bounds.h snort-1.9.1/src/bounds.h
--- snort-1.9.1.orig/src/bounds.h	Wed Dec 31 18:00:00 1969
+++ snort-1.9.1/src/bounds.h	Tue Apr 29 14:06:35 2003
@@ -0,0 +1,127 @@
+#ifndef _BOUNDS_H 
+/*
+** Copyright (C) 2003, Sourcefire, Inc.
+**               Chris Green 
+**
+** This program is free software; you can redistribute it and/or modify
+** it under the terms of the GNU General Public License as published by
+** the Free Software Foundation; either version 2 of the License, or
+** (at your option) any later version.
+**
+** This program is distributed in the hope that it will be useful,
+** but WITHOUT ANY WARRANTY; without even the implied warranty of
+** MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+** GNU General Public License for more details.
+**
+** You should have received a copy of the GNU General Public License
+** along with this program; if not, write to the Free Software
+** Foundation, Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
+**
+*/
+
+

+#ifdef HAVE_CONFIG_H 
+

+#include "snort.h" 
+

+#include <string.h>

+#include <stdio.h>

+
+/* This INLINE is conflicting with the INLINE defined in bitop.h.
+ * So, let's just add a little sanity check here.
+ */

+#ifndef DEBUG 
+    #ifndef INLINE
+        #define INLINE inline
+    #endif
+    #define ERRORRET return 0;

+#else 
+    #ifdef INLINE
+        #undef INLINE
+    #endif
+    #define INLINE
+    #define ERRORRET assert(0==1)

+#endif /* DEBUG */ 
+
+/*

Do you need help?X

+ * Check to make sure that p is less than or equal to the ptr range
+ * pointers
+ *
+ * 1 means it's in bounds, 0 means it's not
+ */
+static INLINE int inBounds(u_int8_t *start, u_int8_t *end, u_int8_t *p)
+{
+    if(p >= start && p < end)
+    {
+        return 1;
+    }
+
+    return 0;
+}
+
+/**
+ * A Safer Memcpy
+ *
+ * @param dst where to copy to
+ * @param src where to copy from
+ * @param n number of bytes to copy
+ * @param start start of the dest buffer
+ * @param end end of the dst buffer
+ *
+ * @return 0 on failure, 1 on success
+ */
+static INLINE int SafeMemcpy(void *dst, void *src, size_t n, void *start, void *end)
+{
+     if(n < 1)
+     {
+         ERRORRET;
+     }
+
+     if(!inBounds(start,end, dst) || !inBounds(start,end,((u_int8_t*)dst)+n))
+     {
+         ERRORRET;
+     }
+
+     memcpy(dst, src, n);
+     return 1;
+}
+
+/**
+ * A Safer *a = *b
+ *
+ * @param start start of the dst buffer
+ * @param end end of the dst buffer
+ * @param dst the location to write to
+ * @param src the source to read from
+ *
+ * @return 0 on failure, 1 on success
+ */
+static INLINE int SafeWrite(u_int8_t *start, u_int8_t *end, u_int8_t *dst, u_int8_t *src)
+{
+    if(!inBounds(start, end, dst))
+    {
+        ERRORRET;
+    }
+
+    *dst = *src;
+    return 1;
+}
+
+static inline int SafeRead(u_int8_t *start, u_int8_t *end, u_int8_t *src, u_int8_t *read)
+{
+    if(!inBounds(start,end, src))
+    {
+        ERRORRET;
+    }
+
+    *read = *start;
+    return 1;
+}
+

+#endif /* _BOUNDS_H */
diff -ruN snort-1.9.1.orig/src/decode.h snort-1.9.1/src/decode.h 
--- snort-1.9.1.orig/src/decode.h	Fri Feb 14 13:32:26 2003
+++ snort-1.9.1/src/decode.h	Tue Apr 29 14:06:35 2003
@@ -165,6 +165,10 @@
 #define UDP_HEADER_LEN          8
 #define ICMP_HEADER_LEN         4

+#ifndef IP_MAXPACKET

+#define IP_MAXPACKET            65535   /* maximum packet size */
+ 
 #define TH_FIN  0x01
 #define TH_SYN  0x02
 #define TH_RST  0x04
diff -ruN snort-1.9.1.orig/src/preprocessors/spp_frag2.c snort-1.9.1/src/preprocessors/spp_frag2.c

Do you need more help?X

--- snort-1.9.1.orig/src/preprocessors/spp_frag2.c	Wed Aug 21 08:02:01 2002
+++ snort-1.9.1/src/preprocessors/spp_frag2.c	Tue Apr 29 14:06:54 2003
@@ -60,6 +60,7 @@
 #include <ctype.h>
 #include <rpc/types.h>

+#include "bounds.h" 

 #include "generators.h"
 #include "log.h"
 #include "detect.h"
@@ -98,6 +99,8 @@
 #define SPARC_TWIDDLE       0

 #endif

+#define DATASIZE (ETHERNET_HEADER_LEN+65536)
+ 

 /*  D A T A   S T R U C T U R E S  **********************************/
 typedef struct _Frag2Data
 {
@@ -302,7 +305,9 @@ 
     if((frag->offset + frag->size) < 65516)
     {
-        memcpy(buf+frag->offset, frag->data, frag->size);
+        if (! SafeMemcpy(buf+frag->offset, frag->data, frag->size,
+                   defrag_pkt->pkt, defrag_pkt->pkt + DATASIZE))
+           ErrorMessage("Unsafe memcpy in frag2, reassembled packet corrupted.\n");
         pc.rebuild_element++;
     }
     else

@@ -779,7 +784,9 @@

     newfrag->data = (u_int8_t *) Frag2Alloc(ft, p->pkth->ts.tv_sec, p->dsize);

  • memcpy(newfrag->data, p->data, p->dsize); + if (! SafeMemcpy(newfrag->data, p->data, p->dsize, + defrag_pkt->pkt, defrag_pkt->pkt + DATASIZE)) + ErrorMessage("Unsafe memcpy in frag2, reassembled packet corrupted.\n"); 
     newfrag->offset = p->frag_offset << 3;
     newfrag->size = p->dsize;
diff -ruN snort-1.9.1.orig/src/preprocessors/spp_stream4.c snort-1.9.1/src/preprocessors/spp_stream4.c
--- snort-1.9.1.orig/src/preprocessors/spp_stream4.c	Fri Feb 14 13:32:27 2003
+++ snort-1.9.1/src/preprocessors/spp_stream4.c	Tue Apr 29 14:08:05 2003
@@ -37,6 +37,17 @@
 #include "config.h"
 #endif
Can we help you?X

+#ifndef DEBUG 

+    #ifndef INLINE
+        #define INLINE inline
+    #endif

+#else 
+    #ifdef INLINE
+        #undef INLINE
+    #endif
+    #define INLINEˇˇˇ

+#endif /* DEBUG */

+ 
 #include 
 #include 
 #include

@@ -65,6 +76,7 @@ 
 #include "generators.h"
 #include "detect.h"
 #include "perf.h"

+#include "bounds.h"

 #include "ubi_SplayTree.h"

@@ -143,6 +155,8 @@
 #define SPARC_TWIDDLE 0
 #endif

+#define MAX_STREAM_SIZE (IP_MAXPACKET - IP_HEADER_LEN - TCP_HEADER_LEN)
+
 /* random array of flush points */

 #define FCOUNT 64
@@ -325,6 +339,7 @@
 void WriteSsnStats(BinStats *);
 void OpenStatsFile();
 static int RetransTooFast(struct timeval *a, struct timeval *b); +static INLINE int isBetween(u_int32_t low, u_int32_t high, u_int32_t cur);

 /*
   Here is where we separate which functions will be called in the @@ -342,6 +357,10 @@ 

+static INLINE int isBetween(u_int32_t low, u_int32_t high, u_int32_t cur)
+{
+        return (cur - low) <= (high - low);
+}

 static int CompareFunc(ubi_trItemPtr ItemPtr, ubi_trNodePtr NodePtr)  {
@@ -462,7 +481,7 @@ 

     /* don't reassemble if we're before the start sequence number or
      * after the last ack'd byte
      */

- if(spd->seq_num < s->base_seq || spd->seq_num > s->last_ack) { + if(!isBetween(s->base_seq, s->last_ack, spd->seq_num)) { 
         DEBUG_WRAP(DebugMessage(DEBUG_STREAM,
                                 "not reassembling because"
                                 " we're (%u) before isn(%u) or after last_ack(%u)\n",

Can't find what you're looking for?X

@@ -471,8 +490,8 @@
     }

     /* if it's in bounds... */
-    if(spd->seq_num >= s->base_seq && spd->seq_num >= s->next_seq &&
-       (spd->seq_num+spd->payload_size) <= s->last_ack)
+    if(isBetween(s->base_seq, s->last_ack, spd->seq_num) &&
+       isBetween(s->base_seq, s->last_ack, (spd->seq_num+spd->payload_size)))
     {
         offset = spd->seq_num - s->base_seq;

@@ -487,16 +506,17 @@
                                 spd->seq_num, s->last_ack, s->base_seq,
                                 spd->payload_size, s->next_seq, offset));

-        memcpy(buf+offset, spd->payload, spd->payload_size);
+        if(! SafeMemcpy(buf+offset, spd->payload, spd->payload_size,
+                   stream_pkt->data, stream_pkt->data + MAX_STREAM_SIZE))
+           ErrorMessage("Unsafe memcpy in stream4, reassembled stream corrupted.\n");

         pc.rebuilt_segs++;

         spd->chuck = 1;
         bd->total_size += spd->payload_size;
     }
-    else if(spd->seq_num >= s->base_seq &&
-            spd->seq_num < s->last_ack &&
-            spd->seq_num + spd->payload_size > s->last_ack)
+    else if(isBetween(s->base_seq, s->last_ack, spd->seq_num) &&
+            ((spd->seq_num + spd->payload_size) > s->last_ack))
     {
         /*
          *  if it starts in bounds and hasn't been completely ack'd,
@@ -518,7 +538,10 @@
             DEBUG_WRAP(DebugMessage(DEBUG_STREAM, "Copying %d bytes into buffer, "
                                     "offset %d, buf %p\n", trunc_size, offset,
                                     buf););
-            memcpy(buf+offset, spd->payload, trunc_size);
+            if (! SafeMemcpy(buf+offset, spd->payload, trunc_size,
+                       stream_pkt->data, stream_pkt->data + MAX_STREAM_SIZE))

Don't know where to look next?X

+                ErrorMessage("Unsafe memcpy in stream4, reassembled stream corrupted.\n");
+
             pc.rebuilt_segs++;
             bd->total_size += trunc_size;
         }

@@ -530,8 +553,7 @@ 
         spd->chuck = 1;
     }
-    else if(spd->seq_num < s->base_seq &&
-            spd->seq_num+spd->payload_size > s->base_seq)
+    else if(isBetween(s->base_seq, s->last_ack, (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
@@ -544,13 +566,16 @@ 
         /* 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););
-            memcpy(buf, spd->payload+offset, trunc_size);
+            if (! SafeMemcpy(buf, spd->payload+offset, trunc_size,
+                       stream_pkt->data, stream_pkt->data + MAX_STREAM_SIZE))
+                ErrorMessage("Unsafe memcpy in stream4, reassembled stream corrupted.\n");
+
             pc.rebuilt_segs++;
             bd->total_size += trunc_size;
         }


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek Welcome to geek heaven.
http://thinkgeek.com/sf

Snort-devel mailing list
Snort-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/snort-devel Received on Tue Apr 29 19:00:28 2003

This archive was generated by hypermail 2.1.8 : Wed Aug 23 2006 - 14:08:05 EDT

Confused? Frustrated?X
Contact Us  Legal Notices  Order Services Online 
Pantek Home  Privacy Policy  IT news  Site Map  Pantek Library