From: Jason Ish <ish@unx.ca>
Date: Tue, 1 Mar 2016 20:36:17 +0000 (-0600)
Subject: dcerpc: fix memory leak when called from smb
X-Git-Tag: suricata-3.0.1RC1~93
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=1efcaf217904b3b2238684a6437d2afb0dec08bf;p=thirdparty%2Fsuricata.git

dcerpc: fix memory leak when called from smb

When DCERPC was wrapped in SMB it wasn't being initialized or
cleaned up properly. To fix, expose DCERPC initialization and
cleanup functions for use by the SMB application layer.

Redmine ticket:
https://redmine.openinfosecfoundation.org/issues/1708
---

diff --git a/src/app-layer-dcerpc.c b/src/app-layer-dcerpc.c
index 44705ea970..9f9862f043 100644
--- a/src/app-layer-dcerpc.c
+++ b/src/app-layer-dcerpc.c
@@ -891,12 +891,10 @@ static uint32_t DCERPCParseBIND(DCERPC *dcerpc, uint8_t *input, uint32_t input_l
                             TAILQ_REMOVE(&dcerpc->dcerpcbindbindack.accepted_uuid_list, item, next);
                             SCFree(item);
                         }
-                        TAILQ_INIT(&dcerpc->dcerpcbindbindack.accepted_uuid_list);
                     }
                     dcerpc->dcerpcbindbindack.uuid_internal_id = 0;
                     dcerpc->dcerpcbindbindack.numctxitems = *(p + 8);
                     dcerpc->dcerpcbindbindack.numctxitemsleft = dcerpc->dcerpcbindbindack.numctxitems;
-                    TAILQ_INIT(&dcerpc->dcerpcbindbindack.uuid_list);
                     dcerpc->bytesprocessed += 12;
                     SCReturnUInt(12U);
                 } else {
@@ -958,12 +956,10 @@ static uint32_t DCERPCParseBIND(DCERPC *dcerpc, uint8_t *input, uint32_t input_l
                         TAILQ_REMOVE(&dcerpc->dcerpcbindbindack.accepted_uuid_list, item, next);
                         SCFree(item);
                     }
-                    TAILQ_INIT(&dcerpc->dcerpcbindbindack.accepted_uuid_list);
                 }
                 dcerpc->dcerpcbindbindack.uuid_internal_id = 0;
                 dcerpc->dcerpcbindbindack.numctxitems = *(p++);
                 dcerpc->dcerpcbindbindack.numctxitemsleft = dcerpc->dcerpcbindbindack.numctxitems;
-                TAILQ_INIT(&dcerpc->dcerpcbindbindack.uuid_list);
                 if (!(--input_len))
                     break;
                 /* fall through */
@@ -1952,49 +1948,59 @@ static int DCERPCParseResponse(Flow *f, void *dcerpc_state,
                        local_data, 1);
 }
 
+void DCERPCInit(DCERPC *dcerpc)
+{
+    dcerpc->transaction_id = 1;
+
+    TAILQ_INIT(&dcerpc->dcerpcbindbindack.uuid_list);
+    TAILQ_INIT(&dcerpc->dcerpcbindbindack.accepted_uuid_list);
+}
+
 static void *DCERPCStateAlloc(void)
 {
     SCEnter();
 
-    DCERPCState *s = SCMalloc(sizeof(DCERPCState));
+    DCERPCState *s = SCCalloc(1, sizeof(DCERPCState));
     if (unlikely(s == NULL)) {
         SCReturnPtr(NULL, "void");
     }
-    memset(s, 0, sizeof(DCERPCState));
 
-    s->dcerpc.transaction_id = 1;
+    DCERPCInit(&s->dcerpc);
 
     SCReturnPtr((void *)s, "void");
 }
 
-static void DCERPCStateFree(void *s)
+void DCERPCCleanup(DCERPC *dcerpc)
 {
-    DCERPCState *sstate = (DCERPCState *) s;
-
-    DCERPCUuidEntry *item;
+    DCERPCUuidEntry *entry;
 
-    while ((item = TAILQ_FIRST(&sstate->dcerpc.dcerpcbindbindack.uuid_list))) {
-        //printUUID("Free", item);
-        TAILQ_REMOVE(&sstate->dcerpc.dcerpcbindbindack.uuid_list, item, next);
-        SCFree(item);
+    while ((entry = TAILQ_FIRST(&dcerpc->dcerpcbindbindack.uuid_list))) {
+        TAILQ_REMOVE(&dcerpc->dcerpcbindbindack.uuid_list, entry, next);
+        SCFree(entry);
     }
 
-    while ((item = TAILQ_FIRST(&sstate->dcerpc.dcerpcbindbindack.accepted_uuid_list))) {
-        //printUUID("Free", item);
-        TAILQ_REMOVE(&sstate->dcerpc.dcerpcbindbindack.accepted_uuid_list, item, next);
-        SCFree(item);
+    while ((entry = TAILQ_FIRST(&dcerpc->dcerpcbindbindack.accepted_uuid_list))) {
+        TAILQ_REMOVE(&dcerpc->dcerpcbindbindack.accepted_uuid_list, entry, next);
+        SCFree(entry);
     }
 
-    if (sstate->dcerpc.dcerpcrequest.stub_data_buffer != NULL) {
-        SCFree(sstate->dcerpc.dcerpcrequest.stub_data_buffer);
-        sstate->dcerpc.dcerpcrequest.stub_data_buffer = NULL;
-        sstate->dcerpc.dcerpcrequest.stub_data_buffer_len = 0;
+    if (dcerpc->dcerpcrequest.stub_data_buffer != NULL) {
+        SCFree(dcerpc->dcerpcrequest.stub_data_buffer);
+        dcerpc->dcerpcrequest.stub_data_buffer = NULL;
+        dcerpc->dcerpcrequest.stub_data_buffer_len = 0;
     }
-    if (sstate->dcerpc.dcerpcresponse.stub_data_buffer != NULL) {
-        SCFree(sstate->dcerpc.dcerpcresponse.stub_data_buffer);
-        sstate->dcerpc.dcerpcresponse.stub_data_buffer = NULL;
-        sstate->dcerpc.dcerpcresponse.stub_data_buffer_len = 0;
+    if (dcerpc->dcerpcresponse.stub_data_buffer != NULL) {
+        SCFree(dcerpc->dcerpcresponse.stub_data_buffer);
+        dcerpc->dcerpcresponse.stub_data_buffer = NULL;
+        dcerpc->dcerpcresponse.stub_data_buffer_len = 0;
     }
+}
+
+static void DCERPCStateFree(void *s)
+{
+    DCERPCState *sstate = (DCERPCState *) s;
+
+    DCERPCCleanup(&sstate->dcerpc);
 
     SCFree(s);
 }
diff --git a/src/app-layer-dcerpc.h b/src/app-layer-dcerpc.h
index 4781f0d1ae..db90461e04 100644
--- a/src/app-layer-dcerpc.h
+++ b/src/app-layer-dcerpc.h
@@ -36,6 +36,8 @@ typedef struct DCERPCState_ {
     uint8_t data_needed_for_dir;
 } DCERPCState;
 
+void DCERPCInit(DCERPC *dcerpc);
+void DCERPCCleanup(DCERPC *dcerpc);
 void RegisterDCERPCParsers(void);
 void DCERPCParserTests(void);
 void DCERPCParserRegisterTests(void);
diff --git a/src/app-layer-smb.c b/src/app-layer-smb.c
index 4d7aa845b7..c56422c214 100644
--- a/src/app-layer-smb.c
+++ b/src/app-layer-smb.c
@@ -42,6 +42,7 @@
 #include "app-layer-detect-proto.h"
 #include "app-layer-protos.h"
 #include "app-layer-parser.h"
+#include "app-layer-dcerpc.h"
 
 #include "util-spm.h"
 #include "util-unittest.h"
@@ -1426,12 +1427,12 @@ static void *SMBStateAlloc(void)
 {
     SCEnter();
 
-    void *s = SCMalloc(sizeof(SMBState));
+    SMBState *s = (SMBState *)SCCalloc(1, sizeof(SMBState));
     if (unlikely(s == NULL)) {
         SCReturnPtr(NULL, "void");
     }
 
-    memset(s, 0, sizeof(SMBState));
+    DCERPCInit(&s->dcerpc);
 
     SCReturnPtr(s, "void");
 }
@@ -1444,23 +1445,7 @@ static void SMBStateFree(void *s)
     SCEnter();
     SMBState *sstate = (SMBState *) s;
 
-    DCERPCUuidEntry *item;
-
-    while ((item = TAILQ_FIRST(&sstate->dcerpc.dcerpcbindbindack.uuid_list))) {
-	//printUUID("Free", item);
-	TAILQ_REMOVE(&sstate->dcerpc.dcerpcbindbindack.uuid_list, item, next);
-	SCFree(item);
-    }
-    if (sstate->dcerpc.dcerpcrequest.stub_data_buffer != NULL) {
-        SCFree(sstate->dcerpc.dcerpcrequest.stub_data_buffer);
-        sstate->dcerpc.dcerpcrequest.stub_data_buffer = NULL;
-        sstate->dcerpc.dcerpcrequest.stub_data_buffer_len = 0;
-    }
-    if (sstate->dcerpc.dcerpcresponse.stub_data_buffer != NULL) {
-        SCFree(sstate->dcerpc.dcerpcresponse.stub_data_buffer);
-        sstate->dcerpc.dcerpcresponse.stub_data_buffer = NULL;
-        sstate->dcerpc.dcerpcresponse.stub_data_buffer_len = 0;
-    }
+    DCERPCCleanup(&sstate->dcerpc);
 
     SCFree(s);
     SCReturn;