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 44705ea970a2b33f3db8a9d131e5bb7709b8cda4..9f9862f043ccd3a823b5977f9612f0c4236b18b2 100644 (file)
--- 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 4781f0d1ae07c3bf9587af630085303c9c0767f8..db90461e044af2604ef690da3f2b5421428d7598 100644 (file)
--- 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 4d7aa845b7d309c987fe7bc34df0ea3241aec5da..c56422c21434a6f33174e314f6791a3924e06b92 100644 (file)
--- 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;