From 1ce1d0e320158985d207b07b3eadde7999890bda Mon Sep 17 00:00:00 2001 From: root Date: Tue, 29 Dec 2009 00:44:23 -0600 Subject: [PATCH] fix for bug #22, better handle malformed smb packets --- src/app-layer-smb.c | 66 +++++++++++++++++++++++++++----------------------- src/app-layer-smb.h | 2 + 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/src/app-layer-smb.c b/src/app-layer-smb.c index 9339eec..3a8c1ba 100644 --- a/src/app-layer-smb.c +++ b/src/app-layer-smb.c @@ -102,6 +102,7 @@ void hexdump(const void *buf, size_t len) { /* For WriteAndX we need to get writeandxdataoffset */ static int SMBParseWriteAndX(Flow *f, void *smb_state, AppLayerParserState *pstate, uint8_t *input, uint32_t input_len, AppLayerParserResult *output) { + SCEnter(); SMBState *sstate = (SMBState *) smb_state; uint8_t *p = input; switch (sstate->andx.andxbytesprocessed) { @@ -121,9 +122,8 @@ static int SMBParseWriteAndX(Flow *f, void *smb_state, AppLayerParserState *psta sstate->andx.dataoffset|= (uint64_t) *(p+25) << 48; sstate->andx.dataoffset|= (uint64_t) *(p+26) << 40; sstate->andx.dataoffset|= (uint64_t) *(p+27) << 32; - input_len -= 28; sstate->bytesprocessed += 28; - return 28; + SCReturnInt(28); } else { sstate->andx.andxcommand = *(p++); if (!(--input_len)) break; @@ -230,10 +230,10 @@ static int SMBParseWriteAndX(Flow *f, void *smb_state, AppLayerParserState *psta break; default: // SHOULD NEVER OCCUR - return 0; + SCReturnInt(-1); } sstate->bytesprocessed += (p - input); - return (p - input); + SCReturnInt(p - input); } /** @@ -241,6 +241,7 @@ static int SMBParseWriteAndX(Flow *f, void *smb_state, AppLayerParserState *psta */ static int SMBParseReadAndX(Flow *f, void *smb_state, AppLayerParserState *pstate, uint8_t *input, uint32_t input_len, AppLayerParserResult *output) { + SCEnter(); SMBState *sstate = (SMBState *) smb_state; uint8_t *p = input; switch (sstate->andx.andxbytesprocessed) { @@ -260,7 +261,7 @@ static int SMBParseReadAndX(Flow *f, void *smb_state, AppLayerParserState *pstat sstate->andx.datalength |= (uint64_t) *(p+17) << 32; input_len -= 24; sstate->bytesprocessed += 24; - return 24; + SCReturnInt(24); } else { sstate->andx.andxcommand = *(p++); if (!(--input_len)) break; @@ -331,11 +332,10 @@ static int SMBParseReadAndX(Flow *f, void *smb_state, AppLayerParserState *pstat break; default: // SHOULD NEVER OCCUR - return 0; + SCReturnInt(0); } - return 0; sstate->bytesprocessed += (p - input); - return (p - input); + SCReturnInt(p - input); } /** @@ -343,6 +343,7 @@ static int SMBParseReadAndX(Flow *f, void *smb_state, AppLayerParserState *pstat */ static int PaddingParser(void *smb_state, AppLayerParserState *pstate, uint8_t *input, uint32_t input_len, AppLayerParserResult *output) { + SCEnter(); SMBState *sstate = (SMBState *) smb_state; uint8_t *p = input; while (sstate->bytesprocessed++ < sstate->andx.dataoffset && sstate->bytecount.bytecount-- && input_len--) { @@ -352,7 +353,7 @@ static int PaddingParser(void *smb_state, AppLayerParserState *pstate, sstate->andx.paddingparsed = 1; } sstate->bytesprocessed += (p - input); - return (p - input); + SCReturnInt(p - input); } /** @@ -361,6 +362,7 @@ static int PaddingParser(void *smb_state, AppLayerParserState *pstate, */ static int DataParser(void *smb_state, AppLayerParserState *pstate, uint8_t *input, uint32_t input_len, AppLayerParserResult *output) { + SCEnter(); SMBState *sstate = (SMBState *) smb_state; uint8_t *p = input; @@ -370,10 +372,9 @@ static int DataParser(void *smb_state, AppLayerParserState *pstate, } } sstate->bytesprocessed += (p - input); - return (p - input); + SCReturnInt(p - input); } - /** * \brief Obtain SMB WordCount which is 2 times the value. * Reset bytecount.bytecountbytes to 0. @@ -386,6 +387,7 @@ static int SMBGetWordCount(Flow *f, void *smb_state, AppLayerParserState *pstate if (input_len) { SMBState *sstate = (SMBState *) smb_state; sstate->wordcount.wordcount = *(input) * 2; + sstate->wordcount.wordcountleft = sstate->wordcount.wordcount; sstate->bytesprocessed++; sstate->bytecount.bytecountbytes = 0; sstate->andx.isandx = isAndX(sstate); @@ -416,6 +418,7 @@ static int SMBGetByteCount(Flow *f, void *smb_state, AppLayerParserState *pstate if (input_len && sstate->bytesprocessed == NBSS_HDR_LEN + SMB_HDR_LEN + 2 + sstate->wordcount.wordcount) { sstate->bytecount.bytecount |= *(p++) << 8; + sstate->bytecount.bytecountleft = sstate->bytecount.bytecount; sstate->bytesprocessed++; SCLogDebug("Bytecount %u", sstate->bytecount.bytecount); --input_len; @@ -439,21 +442,21 @@ static int SMBParseWordCount(Flow *f, void *smb_state, AppLayerParserState *psta retval = SMBParseReadAndX(f, sstate, pstate, input + parsed, input_len, output); parsed += retval; input_len -= retval; - sstate->wordcount.wordcount -= retval; - return retval; + sstate->wordcount.wordcountleft -= retval; + SCReturnInt(retval); } else if (((sstate->smb.flags & SMB_FLAGS_SERVER_TO_REDIR) == 0) && sstate->smb.command == SMB_COM_WRITE_ANDX) { retval = SMBParseWriteAndX(f, sstate, pstate, input + parsed, input_len, output); parsed += retval; input_len -= retval; - sstate->wordcount.wordcount -= retval; - return retval; + sstate->wordcount.wordcountleft -= retval; + SCReturnInt(retval); } else { /* Generic WordCount Handler */ - while (sstate->wordcount.wordcount-- && input_len--) { - printf("0x%02x ", *(p++)); + while (sstate->wordcount.wordcountleft-- && input_len--) { + SCLogDebug("0x%02x wordcountleft %u input_len %u\n", *(p++), + sstate->wordcount.wordcountleft, input_len); } - printf("\n"); + SCLogDebug("\n"); sstate->bytesprocessed += (p - input); - return (p - input); SCReturnInt(p - input); } } @@ -485,20 +488,17 @@ static int SMBParseByteCount(Flow *f, void *smb_state, AppLayerParserState *psta } } - while (sstate->bytecount.bytecount && input_len) { + while (sstate->bytecount.bytecountleft-- && input_len--) { SCLogDebug("0x%02x bytecount %u input_len %u", *(p++), sstate->bytecount.bytecount, input_len); - sstate->wordcount.wordcount--; - input_len--; } - printf("\n"); + SCLogDebug("\n"); sstate->bytesprocessed += (p - input); SCReturnInt(p - input); } -#define DEBUG 1 static int NBSSParseHeader(Flow *f, void *smb_state, AppLayerParserState *pstate, uint8_t *input, uint32_t input_len, AppLayerParserResult *output) { @@ -582,7 +582,6 @@ static int SMBParseHeader(Flow *f, void *smb_state, AppLayerParserState *pstate, sstate->smb.uid |= *(p + 29); sstate->smb.mid = *(p + 30) << 8; sstate->smb.mid |= *(p + 31); - input_len -= SMB_HDR_LEN; sstate->bytesprocessed += SMB_HDR_LEN; SCReturnInt(SMB_HDR_LEN); break; @@ -731,7 +730,7 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, parsed, input_len, output); parsed += retval; input_len -= retval; - printf("SMB Header (%u/%u) Command 0x%02x parsed %u input_len %u\n", + SCLogDebug("SMB Header (%u/%u) Command 0x%02x parsed %u input_len %u\n", sstate->bytesprocessed, NBSS_HDR_LEN + SMB_HDR_LEN, sstate->smb.command, parsed, input_len); } @@ -743,8 +742,6 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, output); parsed += retval; input_len -= retval; - printf("Wordcount (%u) parsed %u input_len %u\n", - sstate->wordcount.wordcount, parsed, input_len); } while (input_len && (sstate->bytesprocessed >= NBSS_HDR_LEN + SMB_HDR_LEN + 1 && @@ -767,6 +764,14 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, input_len -= retval; } + if (SMB_HDR_LEN + sstate->wordcount.wordcount + sstate->bytecount.bytecount + 3 > + sstate->nbss.length) { + printf("Caught Malformed SMB Packet\n"); + pstate->parse_field = 0; + pstate->flags |= APP_LAYER_PARSER_DONE; + SCReturnInt(-1); + } + while (input_len && (sstate->bytesprocessed >= NBSS_HDR_LEN + SMB_HDR_LEN + 3 + sstate->wordcount.wordcount && sstate->bytesprocessed < NBSS_HDR_LEN + SMB_HDR_LEN + 3 @@ -794,6 +799,7 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, */ int isAndX(SMBState *smb_state) { + SCEnter(); switch (smb_state->smb.command) { case SMB_NO_SECONDARY_ANDX_COMMAND: case SMB_COM_LOCKING_ANDX: @@ -805,9 +811,9 @@ int isAndX(SMBState *smb_state) { case SMB_COM_TREE_CONNECT_ANDX: case SMB_COM_NT_CREATE_ANDX: smb_state->andx.andxbytesprocessed = 0; - return 1; + SCReturnInt(1); default: - return 0; + SCReturnInt(0); } } diff --git a/src/app-layer-smb.h b/src/app-layer-smb.h index 169390f..83f0266 100644 --- a/src/app-layer-smb.h +++ b/src/app-layer-smb.h @@ -63,12 +63,14 @@ typedef struct smb_hdr_ { typedef struct wordcount_ { uint8_t wordcount; + uint8_t wordcountleft; uint8_t *words; }wordcount_t, *pwordcount_t; typedef struct bytecount_ { uint8_t bytecountbytes; uint16_t bytecount; + uint16_t bytecountleft; uint8_t *bytes; }bytecount_t, *pbytyecount_t; -- 1.6.5.2