From df3ca322a4fb1ad0f754d7c1e162b7e0bc1aa7d0 Mon Sep 17 00:00:00 2001
From: Victor Julien <victor@inliniac.net>
Date: Fri, 22 Jul 2011 08:42:43 +0200
Subject: [PATCH] Fixes for out of bounds pcre_get_substring calls no longer
 silently accepted by modern pcre.

---
 src/detect-dsize.c | 103 +++++++++++++++++++++++++----------------
 src/detect-ttl.c   | 113 +++++++++++++++++++++++++++------------------
 2 files changed, 129 insertions(+), 87 deletions(-)

diff --git a/src/detect-dsize.c b/src/detect-dsize.c
index 33fab62d72..8ceb23c5c7 100644
--- a/src/detect-dsize.c
+++ b/src/detect-dsize.c
@@ -155,42 +155,54 @@ DetectDsizeData *DetectDsizeParse (char *rawstr)
     mode = (char *)str_ptr;
     SCLogDebug("mode \"%s\"", mode);
 
-    res = pcre_get_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 2, &str_ptr);
-    if (res < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING,"pcre_get_substring failed");
-        goto error;
-    }
-    value1 = (char *)str_ptr;
-    SCLogDebug("value1 \"%s\"", value1);
-
-    res = pcre_get_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 3, &str_ptr);
-    if (res < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING,"pcre_get_substring failed");
-        goto error;
-    }
-    range = (char *)str_ptr;
-    SCLogDebug("range \"%s\"", range);
-
-    res = pcre_get_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 4, &str_ptr);
-    if (res < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING,"pcre_get_substring failed");
-        goto error;
+    if (ret >= 3) {
+        res = pcre_get_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 2, &str_ptr);
+        if (res < 0) {
+            SCLogError(SC_ERR_PCRE_GET_SUBSTRING,"pcre_get_substring failed");
+            goto error;
+        }
+        value1 = (char *)str_ptr;
+        SCLogDebug("value1 \"%s\"", value1);
+
+        if (ret >= 4) {
+            res = pcre_get_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 3, &str_ptr);
+            if (res < 0) {
+                SCLogError(SC_ERR_PCRE_GET_SUBSTRING,"pcre_get_substring failed");
+                goto error;
+            }
+            range = (char *)str_ptr;
+            SCLogDebug("range \"%s\"", range);
+
+            if (ret >= 5) {
+                res = pcre_get_substring((char *)rawstr, ov, MAX_SUBSTRINGS, 4, &str_ptr);
+                if (res < 0) {
+                    SCLogError(SC_ERR_PCRE_GET_SUBSTRING,"pcre_get_substring failed");
+                    goto error;
+                }
+                value2 = (char *)str_ptr;
+                SCLogDebug("value2 \"%s\"", value2);
+            }
+        }
     }
-    value2 = (char *)str_ptr;
-    SCLogDebug("value2 \"%s\"", value2);
 
     dd = SCMalloc(sizeof(DetectDsizeData));
     if (dd == NULL)
         goto error;
     dd->dsize = 0;
     dd->dsize2 = 0;
+    dd->mode = DETECTDSIZE_EQ; // default
 
-    if (mode[0] == '<') dd->mode = DETECTDSIZE_LT;
-    else if (mode[0] == '>') dd->mode = DETECTDSIZE_GT;
-    else dd->mode = DETECTDSIZE_EQ;
+    if (mode != NULL) {
+        if (mode[0] == '<')
+            dd->mode = DETECTDSIZE_LT;
+        else if (mode[0] == '>')
+            dd->mode = DETECTDSIZE_GT;
+        else
+            dd->mode = DETECTDSIZE_EQ;
+    }
 
-    if (strcmp("<>", range) == 0) {
-        if (strlen(mode) != 0) {
+    if (range != NULL && strcmp("<>", range) == 0) {
+        if (mode != NULL && strlen(mode) != 0) {
             SCLogError(SC_ERR_INVALID_ARGUMENT,"Range specified but mode also set");
             goto error;
         }
@@ -198,24 +210,24 @@ DetectDsizeData *DetectDsizeParse (char *rawstr)
     }
 
     /** set the first dsize value */
-    if(ByteExtractStringUint16(&dd->dsize,10,strlen(value1),value1) <= 0){
-        SCLogError(SC_ERR_INVALID_ARGUMENT,"Invalid size value1:\"%s\"",value1);
+    if (ByteExtractStringUint16(&dd->dsize,10,strlen(value1),value1) <= 0) {
+        SCLogError(SC_ERR_INVALID_ARGUMENT, "Invalid size value1:\"%s\"", value1);
         goto error;
     }
 
     /** set the second dsize value if specified */
-    if (strlen(value2) > 0) {
+    if (value2 != NULL && strlen(value2) > 0) {
         if (dd->mode != DETECTDSIZE_RA) {
             SCLogError(SC_ERR_INVALID_ARGUMENT,"Multiple dsize values specified but mode is not range");
             goto error;
         }
 
-        if(ByteExtractStringUint16(&dd->dsize2,10,strlen(value2),value2) <= 0){
+        if (ByteExtractStringUint16(&dd->dsize2,10,strlen(value2),value2) <= 0) {
             SCLogError(SC_ERR_INVALID_ARGUMENT,"Invalid size value2:\"%s\"",value2);
             goto error;
         }
 
-        if (dd->dsize2 <= dd->dsize){
+        if (dd->dsize2 <= dd->dsize) {
             SCLogError(SC_ERR_INVALID_ARGUMENT,"dsize2:%"PRIu16" <= dsize:%"PRIu16"",dd->dsize2,dd->dsize);
             goto error;
         }
@@ -223,18 +235,27 @@ DetectDsizeData *DetectDsizeParse (char *rawstr)
 
     SCLogDebug("dsize parsed succesfully dsize: %"PRIu16" dsize2: %"PRIu16"",dd->dsize,dd->dsize2);
 
-    SCFree(value1);
-    SCFree(value2);
-    SCFree(mode);
-    SCFree(range);
+    if (value1)
+        SCFree(value1);
+    if (value2)
+        SCFree(value2);
+    if (mode)
+        SCFree(mode);
+    if (range)
+        SCFree(range);
     return dd;
 
 error:
-    if (dd) SCFree(dd);
-    if (value1) SCFree(value1);
-    if (value2) SCFree(value2);
-    if (mode) SCFree(mode);
-    if (range) SCFree(range);
+    if (dd)
+        SCFree(dd);
+    if (value1)
+        SCFree(value1);
+    if (value2)
+        SCFree(value2);
+    if (mode)
+        SCFree(mode);
+    if (range)
+        SCFree(range);
     return NULL;
 }
 
diff --git a/src/detect-ttl.c b/src/detect-ttl.c
index e93a8a689c..f860a5ab24 100644
--- a/src/detect-ttl.c
+++ b/src/detect-ttl.c
@@ -156,21 +156,25 @@ DetectTtlData *DetectTtlParse (char *ttlstr) {
     arg1 = (char *) str_ptr;
     SCLogDebug("Arg1 \"%s\"", arg1);
 
-    res = pcre_get_substring((char *) ttlstr, ov, MAX_SUBSTRINGS, 2, &str_ptr);
-    if (res < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
-        goto error;
-    }
-    arg2 = (char *) str_ptr;
-    SCLogDebug("Arg2 \"%s\"", arg2);
-
-    res = pcre_get_substring((char *) ttlstr, ov, MAX_SUBSTRINGS, 3, &str_ptr);
-    if (res < 0) {
-        SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
-        goto error;
+    if (ret >= 3) {
+        res = pcre_get_substring((char *) ttlstr, ov, MAX_SUBSTRINGS, 2, &str_ptr);
+        if (res < 0) {
+            SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+            goto error;
+        }
+        arg2 = (char *) str_ptr;
+        SCLogDebug("Arg2 \"%s\"", arg2);
+
+        if (ret >= 4) {
+            res = pcre_get_substring((char *) ttlstr, ov, MAX_SUBSTRINGS, 3, &str_ptr);
+            if (res < 0) {
+                SCLogError(SC_ERR_PCRE_GET_SUBSTRING, "pcre_get_substring failed");
+                goto error;
+            }
+            arg3 = (char *) str_ptr;
+            SCLogDebug("Arg3 \"%s\"", arg3);
+        }
     }
-    arg3 = (char *) str_ptr;
-    SCLogDebug("Arg3 \"%s\"", arg3);
 
     ttld = SCMalloc(sizeof (DetectTtlData));
     if (ttld == NULL)
@@ -178,47 +182,64 @@ DetectTtlData *DetectTtlParse (char *ttlstr) {
     ttld->ttl1 = 0;
     ttld->ttl2 = 0;
 
-    /*set the values*/
-    switch(arg2[0]) {
-        case '<':
-            ttld->mode = DETECT_TTL_LT;
-            ttld->ttl1 = (uint8_t) atoi(arg3);
+    if (arg2 != NULL) {
+        /*set the values*/
+        switch(arg2[0]) {
+            case '<':
+                if (arg3 == NULL)
+                    goto error;
 
-            SCLogDebug("ttl is %"PRIu8"",ttld->ttl1);
-            if (strlen(arg1) > 0)
-                goto error;
+                ttld->mode = DETECT_TTL_LT;
+                ttld->ttl1 = (uint8_t) atoi(arg3);
 
-            break;
-        case '>':
-            ttld->mode = DETECT_TTL_GT;
-            ttld->ttl1 = (uint8_t) atoi(arg3);
+                SCLogDebug("ttl is %"PRIu8"",ttld->ttl1);
+                if (strlen(arg1) > 0)
+                    goto error;
 
-            SCLogDebug("ttl is %"PRIu8"",ttld->ttl1);
-            if (strlen(arg1) > 0)
-                goto error;
+                break;
+            case '>':
+                if (arg3 == NULL)
+                    goto error;
 
-            break;
-        case '-':
-            if (strlen(arg1)== 0)
-                goto error;
+                ttld->mode = DETECT_TTL_GT;
+                ttld->ttl1 = (uint8_t) atoi(arg3);
 
-            ttld->mode = DETECT_TTL_RA;
-            ttld->ttl1 = (uint8_t) atoi(arg1);
-            if (strlen(arg3) == 0)
-                goto error;
+                SCLogDebug("ttl is %"PRIu8"",ttld->ttl1);
+                if (strlen(arg1) > 0)
+                    goto error;
 
-            ttld->ttl2 = (uint8_t) atoi(arg3);
-            SCLogDebug("ttl is %"PRIu8" and %"PRIu8"",ttld->ttl1, ttld->ttl2);
+                break;
+            case '-':
+                if (arg1 == NULL || strlen(arg1)== 0)
+                    goto error;
+                if (arg3 == NULL || strlen(arg3)== 0)
+                    goto error;
 
-            break;
-        default:
-            ttld->mode = DETECT_TTL_EQ;
+                ttld->mode = DETECT_TTL_RA;
+                ttld->ttl1 = (uint8_t) atoi(arg1);
 
-            if (strlen(arg2) > 0 || strlen(arg3) > 0 || strlen(arg1) == 0)
-                goto error;
+                ttld->ttl2 = (uint8_t) atoi(arg3);
+                SCLogDebug("ttl is %"PRIu8" and %"PRIu8"",ttld->ttl1, ttld->ttl2);
+
+                break;
+            default:
+                ttld->mode = DETECT_TTL_EQ;
+
+                if ((arg2 != NULL && strlen(arg2) > 0) || (arg3 != NULL && strlen(arg3) > 0) || (arg1 == NULL ||strlen(arg1) == 0))
+                    goto error;
+
+                ttld->ttl1 = (uint8_t) atoi(arg1);
+                break;
+        }
+    } else {
+        ttld->mode = DETECT_TTL_EQ;
+
+        if ((arg2 != NULL && strlen(arg2) > 0) ||
+                (arg3 != NULL && strlen(arg3) > 0) ||
+                (arg1 == NULL ||strlen(arg1) == 0))
+            goto error;
 
-            ttld->ttl1 = (uint8_t) atoi(arg1);
-            break;
+        ttld->ttl1 = (uint8_t) atoi(arg1);
     }
 
     SCFree(arg1);