From da63b2d7ab2178ca3f9379ef0e2e1bb9635edb13 Mon Sep 17 00:00:00 2001
From: Kirby Kuehl <kkuehl@gmail.com>
Date: Sun, 7 Feb 2010 12:55:58 -0600
Subject: [PATCH 3/3] fix padding bug

---
 src/app-layer-dcerpc.c |   43 ++++++++++--------
 src/app-layer-smb.c    |  118 +++++++++++++++++++++++++++++++++++++++++++++++-
 2 files changed, 140 insertions(+), 21 deletions(-)

diff --git a/src/app-layer-dcerpc.c b/src/app-layer-dcerpc.c
index eb0760e..e56d005 100644
--- a/src/app-layer-dcerpc.c
+++ b/src/app-layer-dcerpc.c
@@ -102,7 +102,7 @@ void hexdump(const void *buf, size_t len) {
 void printUUID(char *type, struct uuid_entry *uuid) {
     uint8_t i = 0;
     if (uuid == NULL) {
-    	return;
+        return;
     }
     printf("%s UUID [%2u] %s ", type, uuid->ctxid,
             (uuid->result == 0) ? "Accepted" : "Rejected");
@@ -225,7 +225,9 @@ static uint32_t DCERPCParseBINDCTXItem(DCERPC *dcerpc, uint8_t *input, uint32_t
                             dcerpc->dcerpcbindbindack.uuid_entry->versionminor = dcerpc->dcerpcbindbindack.versionminor;
                             TAILQ_INSERT_HEAD(&dcerpc->dcerpcbindbindack.uuid_list, dcerpc->dcerpcbindbindack.uuid_entry,
                                     next);
-                            //printUUID("BIND", dcerpc->dcerpcbindbindack.uuid_entry);
+#ifdef UNITTESTS
+                            printUUID("BIND", dcerpc->dcerpcbindbindack.uuid_entry);
+#endif
                             dcerpc->dcerpcbindbindack.numctxitemsleft--;
                             dcerpc->bytesprocessed += (44);
                             dcerpc->dcerpcbindbindack.ctxbytesprocessed += (44);
@@ -427,7 +429,9 @@ static uint32_t DCERPCParseBINDCTXItem(DCERPC *dcerpc, uint8_t *input, uint32_t
                         dcerpc->dcerpcbindbindack.uuid_entry->versionminor = dcerpc->dcerpcbindbindack.versionminor;
                         TAILQ_INSERT_HEAD(&dcerpc->dcerpcbindbindack.uuid_list, dcerpc->dcerpcbindbindack.uuid_entry,
                                 next);
-                        //printUUID("BIND", dcerpc->dcerpcbindbindack.uuid_entry);
+#ifdef UNITTESTS
+                        printUUID("BIND", dcerpc->dcerpcbindbindack.uuid_entry);
+#endif
                         dcerpc->dcerpcbindbindack.numctxitemsleft--;
                         dcerpc->bytesprocessed += (44);
                         dcerpc->dcerpcbindbindack.ctxbytesprocessed += (44);
@@ -466,7 +470,9 @@ static uint32_t DCERPCParseBINDACKCTXItem(DCERPC *dcerpc, uint8_t *input, uint32
                         if (uuid_entry->ctxid == dcerpc->dcerpcbindbindack.numctxitems
                                 - dcerpc->dcerpcbindbindack.numctxitemsleft) {
                             uuid_entry->result = dcerpc->dcerpcbindbindack.result;
-                            //printUUID("BIND_ACK", uuid_entry);
+#ifdef UNITTESTS
+                            printUUID("BIND_ACK", uuid_entry);
+#endif
                             break;
                         }
                     }
@@ -574,7 +580,9 @@ static uint32_t DCERPCParseBINDACKCTXItem(DCERPC *dcerpc, uint8_t *input, uint32
                     if (uuid_entry->ctxid == dcerpc->dcerpcbindbindack.numctxitems
                             - dcerpc->dcerpcbindbindack.numctxitemsleft) {
                         uuid_entry->result = dcerpc->dcerpcbindbindack.result;
-                        //printUUID("BIND_ACK", uuid_entry);
+#ifdef UNITTESTS
+                        printUUID("BIND_ACK", uuid_entry);
+#endif
                         break;
                     }
                 }
@@ -941,18 +949,18 @@ static uint32_t DCERPCParseHeader(DCERPC *dcerpc, uint8_t *input, uint32_t input
                 if (!(--input_len))
                     break;
             case 14:
-                        dcerpc->dcerpchdr.call_id |= *(p++) << 8;
-                        if (!(--input_len))
-                            break;
+                dcerpc->dcerpchdr.call_id |= *(p++) << 8;
+                if (!(--input_len))
+                    break;
             case 15:
-                        dcerpc->dcerpchdr.call_id |= *(p++);
-                        if (dcerpc->dcerpchdr.packed_drep[0] == 0x01) {
-                            SCByteSwap16(dcerpc->dcerpchdr.frag_length);
-                            SCByteSwap16(dcerpc->dcerpchdr.auth_length);
-                            SCByteSwap32(dcerpc->dcerpchdr.call_id);
-                        }
-                        --input_len;
-                        break;
+                dcerpc->dcerpchdr.call_id |= *(p++);
+                if (dcerpc->dcerpchdr.packed_drep[0] == 0x01) {
+                    SCByteSwap16(dcerpc->dcerpchdr.frag_length);
+                    SCByteSwap16(dcerpc->dcerpchdr.auth_length);
+                    SCByteSwap32(dcerpc->dcerpchdr.call_id);
+                }
+                --input_len;
+                break;
         }
     }
     dcerpc->bytesprocessed += (p - input);
@@ -1660,7 +1668,6 @@ int DCERPCParserTest01(void) {
         printUUID("BIND_ACK", uuid_entry);
     }
 
-    //hexdump(dcerpcrequest, requestlen);
     r = AppLayerParse(&f, ALPROTO_DCERPC, STREAM_TOSERVER|STREAM_EOF, dcerpcrequest, requestlen);
     if (r != 0) {
         printf("dcerpc header check returned %" PRId32 ", expected 0: ", r);
@@ -1814,7 +1821,6 @@ int DCERPCParserTest02(void) {
     uint32_t requestlen = sizeof(dcerpcrequest);
 
     TcpSession ssn;
-    //struct uuid_entry *uuid_entry;
 
     memset(&f, 0, sizeof(f));
     memset(&ssn, 0, sizeof(ssn));
@@ -2002,7 +2008,6 @@ int DCERPCParserTest03(void) {
     uint32_t requestlen = sizeof(dcerpcrequest);
 
     TcpSession ssn;
-    //struct uuid_entry *uuid_entry;
 
     memset(&f, 0, sizeof(f));
     memset(&ssn, 0, sizeof(ssn));
diff --git a/src/app-layer-smb.c b/src/app-layer-smb.c
index f0bbaad..46f2cb3 100644
--- a/src/app-layer-smb.c
+++ b/src/app-layer-smb.c
@@ -507,7 +507,7 @@ static uint32_t PaddingParser(void *smb_state, AppLayerParserState *pstate,
     SMBState *sstate = (SMBState *) smb_state;
     uint8_t *p = input;
     /* Check for validity of dataoffset */
-    if (sstate->bytesprocessed > sstate->andx.dataoffset) {
+    if ((sstate->bytesprocessed - NBSS_HDR_LEN) > sstate->andx.dataoffset) {
         sstate->andx.paddingparsed = 1;
         SCReturnUInt((uint32_t)(p - input));
     }
@@ -1034,11 +1034,13 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate,
 
             } while (sstate->andx.andxcommand != SMB_NO_SECONDARY_ANDX_COMMAND
                     && input_len);
-            if (sstate->bytesprocessed == sstate->nbss.length + NBSS_HDR_LEN) {
+
+            if (sstate->bytesprocessed >= sstate->nbss.length + NBSS_HDR_LEN) {
             	sstate->bytesprocessed = 0;
             }
             break;
         default:
+        	sstate->bytesprocessed = 0;
             break;
     }
     pstate->parse_field = 0;
@@ -1519,6 +1521,117 @@ int SMBParserTest03(void) {
 end:
     return result;
 }
+
+int SMBParserTest04(void) {
+    int result = 1;
+    Flow f;
+    uint8_t smbbuf1[] = {
+    0x00, 0x00, 0x00, 0x88, 0xff, 0x53, 0x4d, 0x42,
+    0x2f, 0x00, 0x00, 0x00, 0x00, 0x18, 0x07, 0xc8,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x7c, 0x05,
+    0x00, 0x08, 0x00, 0x00, 0x0e, 0xff, 0x00, 0x00,
+    0x00, 0x00, 0x40, 0x00, 0x00, 0x00, 0x00, 0xff,
+    0x00, 0x00, 0x00, 0x08, 0x00, 0x48, 0x00, 0x00,
+    0x00, 0x48, 0x00, 0x40, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x49, 0x00, 0xab, 0x05, 0x00, 0x0b, 0x03,
+    0x10, 0x00, 0x00, 0x00, 0x48, 0x00, 0x00, 0x00,
+    0x01, 0x00, 0x00, 0x00, 0xd0, 0x16, 0xd0, 0x16,
+    0x00, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x01, 0x00, 0x78, 0x56, 0x34, 0x12,
+    0x34, 0x12, 0xcd, 0xab, 0xef, 0x00, 0x01, 0x23,
+    0x45, 0x67, 0x89, 0xab, 0x01, 0x00, 0x00, 0x00,
+    0x04, 0x5d, 0x88, 0x8a, 0xeb, 0x1c, 0xc9, 0x11,
+    0x9f, 0xe8, 0x08, 0x00, 0x2b, 0x10, 0x48, 0x60,
+    0x02, 0x00, 0x00, 0x00 };
+    uint8_t smbbuf2[] = {
+    0x00, 0x00, 0x00, 0x2f, 0xff, 0x53, 0x4d, 0x42,
+    0x2f, 0x00, 0x00, 0x00, 0x00, 0x98, 0x07, 0xc8,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x7c, 0x05,
+    0x00, 0x08, 0x00, 0x00, 0x06, 0xff, 0x00, 0x2f,
+    0x00, 0x48, 0x00, 0xff, 0xff, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00 };
+    uint8_t smbbuf3[] = {
+    0x00, 0x00, 0x00, 0x3b, 0xff, 0x53, 0x4d, 0x42,
+    0x2e, 0x00, 0x00, 0x00, 0x00, 0x18, 0x03, 0xc8,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x7c, 0x05,
+    0x00, 0x08, 0x00, 0x00, 0x0c, 0xff, 0x00, 0xde,
+    0xde, 0x00, 0x40, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x04, 0x00, 0x04, 0xff, 0xff, 0xff, 0xff, 0x00,
+    0x04, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00 };
+    uint8_t smbbuf4[] = {
+    0x00, 0x00, 0x00, 0x80, 0xff, 0x53, 0x4d, 0x42,
+    0x2e, 0x00, 0x00, 0x00, 0x00, 0x98, 0x03, 0xc8,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x08, 0x7c, 0x05,
+    0x00, 0x08, 0x00, 0x00, 0x0c, 0xff, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x44,
+    0x00, 0x3c, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x00, 0x00, 0x00, 0x00, 0x00, 0x45, 0x00, 0x00,
+    0x05, 0x00, 0x0c, 0x03, 0x10, 0x00, 0x00, 0x00,
+    0x44, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00,
+    0xb8, 0x10, 0xb8, 0x10, 0x5d, 0xe0, 0x00, 0x00,
+    0x0e, 0x00, 0x5c, 0x70, 0x69, 0x70, 0x65, 0x5c,
+    0x73, 0x70, 0x6f, 0x6f, 0x6c, 0x73, 0x73, 0x00,
+    0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,
+    0x04, 0x5d, 0x88, 0x8a, 0xeb, 0x1c, 0xc9, 0x11,
+    0x9f, 0xe8, 0x08, 0x00, 0x2b, 0x10, 0x48, 0x60,
+    0x02, 0x00, 0x00, 0x00 };
+    uint32_t smblen1 = sizeof(smbbuf1);
+    uint32_t smblen2 = sizeof(smbbuf2);
+    uint32_t smblen3 = sizeof(smbbuf3);
+    uint32_t smblen4 = sizeof(smbbuf4);
+    TcpSession ssn;
+    int r = 0;
+    memset(&f, 0, sizeof(f));
+    memset(&ssn, 0, sizeof(ssn));
+    StreamL7DataPtrInit(&ssn,StreamL7GetStorageSize());
+    f.protoctx = (void *)&ssn;
+
+    r = AppLayerParse(&f, ALPROTO_SMB, STREAM_TOSERVER|STREAM_START, smbbuf1, smblen1);
+    if (r != 0) {
+        printf("smb header check returned %" PRId32 ", expected 0: ", r);
+        result = 0;
+        goto end;
+    }
+
+    SMBState *smb_state = ssn.aldata[AlpGetStateIdx(ALPROTO_SMB)];
+    if (smb_state == NULL) {
+        printf("no smb state: ");
+        result = 0;
+        goto end;
+    }
+
+    if (smb_state->smb.command != SMB_COM_WRITE_ANDX) {
+        printf("expected SMB command 0x%02x , got 0x%02x : ", SMB_COM_WRITE_ANDX, smb_state->smb.command);
+        result = 0;
+        goto end;
+    }
+
+    r = AppLayerParse(&f, ALPROTO_SMB, STREAM_TOSERVER, smbbuf2, smblen2);
+    if (r != 0) {
+        printf("smb header check returned %" PRId32 ", expected 0: ", r);
+        result = 0;
+        goto end;
+    }
+    r = AppLayerParse(&f, ALPROTO_SMB, STREAM_TOSERVER, smbbuf3, smblen3);
+    if (r != 0) {
+        printf("smb header check returned %" PRId32 ", expected 0: ", r);
+        result = 0;
+        goto end;
+    }
+    r = AppLayerParse(&f, ALPROTO_SMB, STREAM_TOSERVER, smbbuf4, smblen4);
+    if (r != 0) {
+        printf("smb header check returned %" PRId32 ", expected 0: ", r);
+        result = 0;
+        goto end;
+    }
+end:
+    return result;
+}
+
 #endif
 
 void SMBParserRegisterTests(void) {
@@ -1527,6 +1640,7 @@ void SMBParserRegisterTests(void) {
     UtRegisterTest("SMBParserTest01", SMBParserTest01, 1);
     UtRegisterTest("SMBParserTest02", SMBParserTest02, 1);
     UtRegisterTest("SMBParserTest03", SMBParserTest03, 1);
+    UtRegisterTest("SMBParserTest04", SMBParserTest04, 1);
 #endif
 }
 
-- 
1.6.6