From 73c6fb16ba59945d529d262324d1129bd8429d39 Mon Sep 17 00:00:00 2001 From: root Date: Thu, 6 May 2010 12:32:51 -0500 Subject: [PATCH] Return 0 instead of -1 when SMB and DCERPC encounter non fatal errors to clean up errors emitted in AppLayerParse. --- src/app-layer-dcerpc-common.h | 2 +- src/app-layer-dcerpc.c | 77 ++++++++++++++++++++++--------- src/app-layer-smb.c | 87 +++++++++++++++++++++-------------- 3 files changed, 110 insertions(+), 56 deletions(-) diff --git a/src/app-layer-dcerpc-common.h b/src/app-layer-dcerpc-common.h index b5f19247df..ac634a3896 100644 --- a/src/app-layer-dcerpc-common.h +++ b/src/app-layer-dcerpc-common.h @@ -176,7 +176,7 @@ typedef struct DCERPC_ { #define NO_PSAP_AVAILABLE 7 /* not used */ int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len); -void hexdump(const void *buf, size_t len); +void hexdump(Flow *f, const void *buf, size_t len); void printUUID(char *type, DCERPCUuidEntry *uuid); #endif /* __APP_LAYER_DCERPC_COMMON_H__ */ diff --git a/src/app-layer-dcerpc.c b/src/app-layer-dcerpc.c index d54a855ef4..bcd0887a37 100644 --- a/src/app-layer-dcerpc.c +++ b/src/app-layer-dcerpc.c @@ -58,13 +58,30 @@ enum { }; /* \brief hexdump function from libdnet, used for debugging only */ -void hexdump(const void *buf, size_t len) { +void hexdump(Flow *f, const void *buf, size_t len) { /* dumps len bytes of *buf to stdout. Looks like: * [0000] 75 6E 6B 6E 6F 77 6E 20 * 30 FF 00 00 00 00 39 00 unknown 0.....9. * (in a single line of course) */ - + if (f->src.family == AF_INET) { + char src[16]; + char dst[16]; + inet_ntop(AF_INET, (const void*)&f->src.addr_data32[0], src, + sizeof (src)); + inet_ntop(AF_INET, (const void*)&f->dst.addr_data32[0], dst, + sizeof (dst)); + printf("%s:%d -> %s:%d\n", src, f->sp, dst, f->dp); + } else { + char dst6[46]; + char src6[46]; + inet_ntop(AF_INET6, (const void*)&f->src.addr_data32, src6, + sizeof (src6)); + inet_ntop(AF_INET6, (const void*)&f->dst.addr_data32, dst6, + sizeof (dst6)); + printf("%s:%d -> %s:%d\n", src6, f->sp, dst6, f->dp); + + } const unsigned char *p = buf; unsigned char c; size_t n; @@ -237,6 +254,7 @@ static uint32_t DCERPCParseBINDCTXItem(DCERPC *dcerpc, uint8_t *input, uint32_t dcerpc->dcerpcbindbindack.uuid_entry = (DCERPCUuidEntry *) SCCalloc(1, sizeof(DCERPCUuidEntry)); if (dcerpc->dcerpcbindbindack.uuid_entry == NULL) { + SCLogDebug("UUID Entry is NULL\n"); SCReturnUInt(0); } else { memcpy(dcerpc->dcerpcbindbindack.uuid_entry->uuid, dcerpc->dcerpcbindbindack.uuid, @@ -443,6 +461,7 @@ static uint32_t DCERPCParseBINDCTXItem(DCERPC *dcerpc, uint8_t *input, uint32_t dcerpc->dcerpcbindbindack.uuid_entry = (DCERPCUuidEntry *) SCCalloc(1, sizeof(DCERPCUuidEntry)); if (dcerpc->dcerpcbindbindack.uuid_entry == NULL) { + SCLogDebug("UUID Entry is NULL\n"); SCReturnUInt(0); } else { memcpy(dcerpc->dcerpcbindbindack.uuid_entry->uuid, dcerpc->dcerpcbindbindack.uuid, @@ -702,6 +721,10 @@ static uint32_t DCERPCParseBIND(DCERPC *dcerpc, uint8_t *input, uint32_t input_l p++; --input_len; break; + default: + dcerpc->bytesprocessed++; + SCReturnUInt(1); + break; } } dcerpc->bytesprocessed += (p - input); @@ -781,6 +804,10 @@ static uint32_t DCERPCParseBINDACK(DCERPC *dcerpc, uint8_t *input, uint32_t inpu dcerpc->dcerpcbindbindack.secondaryaddrlen); --input_len; break; + default: + dcerpc->bytesprocessed++; + SCReturnUInt(1); + break; } dcerpc->bytesprocessed += (p - input); SCReturnUInt((uint32_t)(p - input)); @@ -855,6 +882,10 @@ static uint32_t DCERPCParseREQUEST(DCERPC *dcerpc, uint8_t *input, uint32_t inpu } --input_len; break; + default: + dcerpc->bytesprocessed++; + SCReturnUInt(1); + break; } dcerpc->bytesprocessed += (p - input); SCReturnUInt((uint32_t)(p - input)); @@ -892,8 +923,8 @@ static int DCERPCParseHeader(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) if ((dcerpc->dcerpchdr.rpc_vers != 5) || ((dcerpc->dcerpchdr.rpc_vers_minor != 0) && (dcerpc->dcerpchdr.rpc_vers_minor != 1))) { - SCLogDebug("DCERPC Header did not validate"); - SCReturnInt(-1); + SCLogDebug("DCERPC Header did not validate"); + SCReturnInt(-1); } dcerpc->dcerpchdr.type = *(p + 2); dcerpc->dcerpchdr.pfc_flags = *(p + 3); @@ -933,8 +964,8 @@ static int DCERPCParseHeader(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) if ((dcerpc->dcerpchdr.rpc_vers != 5) || ((dcerpc->dcerpchdr.rpc_vers_minor != 0) && (dcerpc->dcerpchdr.rpc_vers_minor != 1))) { - SCLogDebug("DCERPC Header did not validate"); - SCReturnInt(-1); + SCLogDebug("DCERPC Header did not validate"); + SCReturnInt(-1); } if (!(--input_len)) break; @@ -999,6 +1030,9 @@ static int DCERPCParseHeader(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) } --input_len; break; + default: + dcerpc->bytesprocessed++; + SCReturnInt(1); } } dcerpc->bytesprocessed += (p - input); @@ -1015,7 +1049,7 @@ int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { hdrretval = DCERPCParseHeader(dcerpc, input, input_len); if (hdrretval == -1) { dcerpc->bytesprocessed = 0; - SCReturnInt(hdrretval); + SCReturnInt(0); } else { parsed += hdrretval; input_len -= hdrretval; @@ -1050,8 +1084,8 @@ int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { parsed += retval; input_len -= retval; } else if (input_len) { - SCLogDebug("Error Parsing DCERPC BIND"); - parsed -= input_len; + SCLogDebug("Error Parsing DCERPC %s\n", (dcerpc->dcerpchdr.type == BIND) ? "BIND" : "ALTER_CONTEXT"); + parsed = 0; input_len = 0; } } @@ -1073,8 +1107,9 @@ int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { dcerpc->dcerpchdr.frag_length, dcerpc->dcerpcbindbindack.numctxitemsleft, dcerpc->dcerpcbindbindack.numctxitems); } else if (input_len) { - SCLogDebug("Error Parsing CTX Item"); - parsed -= input_len; + //parsed -= input_len; + parsed = 0; + SCLogDebug("Error Parsing CTX Item %u\n", parsed); input_len = 0; dcerpc->dcerpcbindbindack.numctxitemsleft = 0; } @@ -1096,8 +1131,8 @@ int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { SCLogDebug("DCERPCParseBINDACK processed %u/%u left %u\n", dcerpc->bytesprocessed, dcerpc->dcerpchdr.frag_length, input_len); } else if (input_len) { - SCLogDebug("Error parsing BIND_ACK"); - parsed -= input_len; + SCLogDebug("Error parsing %s\n", (dcerpc->dcerpchdr.type == BIND_ACK) ? "BIND_ACK" : "ALTER_CONTEXT_RESP"); + parsed = 0; input_len = 0; } } @@ -1115,7 +1150,7 @@ int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { dcerpc->dcerpcbindbindack.secondaryaddrlen); } else if (input_len) { SCLogDebug("Error parsing Secondary Address"); - parsed -= input_len; + parsed = 0; input_len = 0; } } @@ -1140,7 +1175,7 @@ int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { dcerpc->pad); } else if (input_len) { SCLogDebug("Error parsing DCERPC Padding"); - parsed -= input_len; + parsed = 0; input_len = 0; } } @@ -1157,7 +1192,7 @@ int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { dcerpc->dcerpchdr.frag_length, dcerpc->dcerpcbindbindack.numctxitems); } else if (input_len) { SCLogDebug("Error parsing CTX Items"); - parsed -= input_len; + parsed = 0; input_len = 0; } } @@ -1178,7 +1213,7 @@ int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { input_len -= retval; } else if (input_len) { SCLogDebug("Error parsing CTX Items"); - parsed -= input_len; + parsed = 0; input_len = 0; dcerpc->dcerpcbindbindack.numctxitemsleft = 0; @@ -1202,8 +1237,8 @@ int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { input_len -= retval; dcerpc->padleft = dcerpc->dcerpchdr.frag_length - dcerpc->bytesprocessed; } else if (input_len) { - SCLogDebug("Error parsing DCERPC Request"); - parsed -= input_len; + SCLogDebug("Error parsing DCERPC %s\n", (dcerpc->dcerpchdr.type == REQUEST) ? "REQUEST" : "RESPONSE"); + parsed = 0; dcerpc->padleft = 0; input_len = 0; } @@ -1217,7 +1252,7 @@ int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { input_len -= retval; } else if (input_len) { SCLogDebug("Error parsing DCERPC Stub Data"); - parsed -= input_len; + parsed = 0; input_len = 0; dcerpc->bytesprocessed = 0; } @@ -1246,7 +1281,7 @@ static int DCERPCParse(Flow *f, void *dcerpc_state, retval = DCERPCParser(&sstate->dcerpc, input, input_len); if (retval == -1) { - SCReturnInt(-1); + SCReturnInt(0); } if (pstate == NULL) SCReturnInt(-1); diff --git a/src/app-layer-smb.c b/src/app-layer-smb.c index a68fb6e8ce..e5c67a8769 100644 --- a/src/app-layer-smb.c +++ b/src/app-layer-smb.c @@ -217,6 +217,10 @@ static uint32_t SMBParseWriteAndX(Flow *f, void *smb_state, sstate->andx.dataoffset |= (uint64_t) *(p++) << 32; --input_len; break; + default: + sstate->bytesprocessed++; + SCReturnUInt(1); + break; } sstate->bytesprocessed += (p - input); SCReturnUInt((uint32_t)(p - input)); @@ -334,6 +338,11 @@ static uint32_t SMBParseReadAndX(Flow *f, void *smb_state, p++; --input_len; break; + default: + sstate->bytesprocessed++; + SCReturnUInt(1); + break; + } sstate->bytesprocessed += (p - input); SCReturnUInt((uint32_t)(p - input)); @@ -511,6 +520,13 @@ static uint32_t SMBParseTransact(Flow *f, void *smb_state, p++; --input_len; break; + default: + SCLogDebug("SMB_COM_TRANSACTION AndX bytes processed is greater than 31 %u\n", sstate->andx.andxbytesprocessed); + sstate->bytesprocessed++; + sstate->andx.andxbytesprocessed++; + SCReturnUInt(1); + break; + } sstate->bytesprocessed += (p - input); sstate->andx.andxbytesprocessed += (p - input); @@ -628,32 +644,25 @@ static uint32_t SMBParseWordCount(Flow *f, void *smb_state, SMBState *sstate = (SMBState *) smb_state; uint8_t *p = input; uint32_t retval = 0; - uint32_t parsed = 0; if ((sstate->smb.flags & SMB_FLAGS_SERVER_TO_REDIR) && sstate->smb.command == SMB_COM_READ_ANDX) { - retval = SMBParseReadAndX(f, sstate, pstate, input + parsed, input_len, + retval = SMBParseReadAndX(f, sstate, pstate, input, input_len, output); - parsed += retval; - input_len -= retval; sstate->wordcount.wordcountleft -= retval; - SCLogDebug("SMB_COM_READ_ANDX %u bytes at offset %"PRIu64"\n", sstate->andx.datalength, sstate->andx.dataoffset); + SCLogDebug("SMB_COM_READ_ANDX returned %d - %u bytes at offset %"PRIu64"\n", retval, sstate->andx.datalength, sstate->andx.dataoffset); SCReturnUInt(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, + retval = SMBParseWriteAndX(f, sstate, pstate, input, input_len, output); - parsed += retval; - input_len -= retval; sstate->wordcount.wordcountleft -= retval; - SCLogDebug("SMB_COM_WRITE_ANDX %u bytes at offset %"PRIu64"\n", sstate->andx.datalength, sstate->andx.dataoffset); + SCLogDebug("SMB_COM_WRITE_ANDX returned %d - %u bytes at offset %"PRIu64"\n", retval, sstate->andx.datalength, sstate->andx.dataoffset); SCReturnUInt(retval); } else if (sstate->smb.command == SMB_COM_TRANSACTION) { - retval = SMBParseTransact(f, sstate, pstate, input + parsed, input_len, + retval = SMBParseTransact(f, sstate, pstate, input, input_len, output); - parsed += retval; - input_len -= retval; sstate->wordcount.wordcountleft -= retval; - SCLogDebug("SMB_COM_TRANSACTION %u bytes at offset %"PRIu64"\n", sstate->andx.datalength, sstate->andx.dataoffset); + SCLogDebug("SMB_COM_TRANSACTION returned %d - %u bytes at offset %"PRIu64"\n", retval, sstate->andx.datalength, sstate->andx.dataoffset); SCReturnUInt(retval); } else { /* Generic WordCount Handler */ while (sstate->wordcount.wordcountleft-- && input_len--) { @@ -702,10 +711,10 @@ static uint32_t SMBParseByteCount(Flow *f, void *smb_state, input_len -= (uint32_t)sres; } else { /* Did not Validate as DCERPC over SMB */ while (sstate->bytecount.bytecountleft-- && input_len--) { - SCLogDebug("0x%02x bytecount %"PRIu16"/%"PRIu16" input_len %"PRIu32, *p, + SCLogDebug("0x%02x bytecount %"PRIu16"/%"PRIu16" input_len %"PRIu32, *p, sstate->bytecount.bytecountleft, sstate->bytecount.bytecount, input_len); - p++; + p++; } sstate->bytesprocessed += (p - input); SCReturnUInt((p - input)); @@ -765,6 +774,7 @@ static uint32_t NBSSParseHeader(Flow *f, void *smb_state, } sstate->bytesprocessed += (p - input); } + if ((p - input < 0)) SCReturnUInt((uint32_t)(p - input)); } @@ -815,24 +825,32 @@ static int SMBParseHeader(Flow *f, void *smb_state, SCReturnInt(32); break; } else { - if (*(p++) != 0xff) + if (*(p++) != 0xff) { + SCLogDebug("SMB Header did not validate"); SCReturnInt(-1); + } if (!(--input_len)) break; } case 5: - if (*(p++) != 'S') + if (*(p++) != 'S') { + SCLogDebug("SMB Header did not validate"); SCReturnInt(-1); + } if (!(--input_len)) break; case 6: - if (*(p++) != 'M') + if (*(p++) != 'M') { + SCLogDebug("SMB Header did not validate"); SCReturnInt(-1); + } if (!(--input_len)) break; case 7: - if (*(p++) != 'B') + if (*(p++) != 'B') { + SCLogDebug("SMB Header did not validate"); SCReturnInt(-1); + } if (!(--input_len)) break; case 8: @@ -963,7 +981,8 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, int hdrretval = 0; if (pstate == NULL) { - SCReturnInt(-1); + SCLogDebug("pstate == NULL"); + SCReturnInt(0); } while (input_len && sstate->bytesprocessed < NBSS_HDR_LEN) { retval @@ -973,13 +992,13 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, parsed += retval; input_len -= retval; SCLogDebug( - "[1] NBSS Header (%u/%u) Type 0x%02x Length 0x%04x parsed %"PRIu64" input_len %u", + "[1] NBSS Header (%u/%u) Type 0x%02x Length 0x%04x parsed %"PRIu64" input_len %u\n", sstate->bytesprocessed, NBSS_HDR_LEN, sstate->nbss.type, sstate->nbss.length, parsed, input_len); } else if (input_len) { SCLogDebug("Error parsing NBSS Header\n"); sstate->bytesprocessed = 0; - SCReturnInt(-1); + SCReturnInt(0); } } @@ -992,12 +1011,12 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, if (hdrretval == -1) { SCLogDebug("Error parsing SMB Header\n"); sstate->bytesprocessed = 0; - SCReturnInt(hdrretval); + SCReturnInt(0); } else { parsed += hdrretval; input_len -= hdrretval; SCLogDebug( - "[2] SMB Header (%u/%u) Command 0x%02x parsed %"PRIu64" input_len %u", + "[2] SMB Header (%u/%u) Command 0x%02x parsed %"PRIu64" input_len %u\n", sstate->bytesprocessed, NBSS_HDR_LEN + SMB_HDR_LEN, sstate->smb.command, parsed, input_len); } @@ -1014,12 +1033,12 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, } else if (input_len) { SCLogDebug("Error parsing SMB Word Count\n"); sstate->bytesprocessed = 0; - SCReturnInt(-1); + SCReturnInt(0); } - SCLogDebug("[3] WordCount (%u/%u) WordCount %u parsed %"PRIu64" input_len %u", + SCLogDebug("[3] WordCount (%u/%u) WordCount %u parsed %"PRIu64" input_len %u\n", sstate->bytesprocessed, NBSS_HDR_LEN + SMB_HDR_LEN + 1, - sstate->wordcount.wordcount, - parsed, input_len); + sstate->wordcount.wordcount, + parsed, input_len); } while (input_len && (sstate->bytesprocessed >= NBSS_HDR_LEN @@ -1031,11 +1050,11 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, parsed += retval; input_len -= retval; } else if (input_len) { - SCLogDebug("Error parsing SMB Word Count Data\n"); + SCLogDebug("Error parsing SMB Word Count Data retval %"PRIu64" input_len %u\n", retval, input_len); sstate->bytesprocessed = 0; - SCReturnInt(-1); + SCReturnInt(0); } - SCLogDebug("[4] Parsing WordCount (%u/%u) WordCount %u parsed %"PRIu64" input_len %u", + SCLogDebug("[4] Parsing WordCount (%u/%u) WordCount %u parsed %"PRIu64" input_len %u\n", sstate->bytesprocessed, NBSS_HDR_LEN + SMB_HDR_LEN + 1 + sstate->wordcount.wordcount, sstate->wordcount.wordcount, parsed, input_len); @@ -1053,7 +1072,7 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, } else if (input_len) { SCLogDebug("Error parsing SMB Byte Count\n"); sstate->bytesprocessed = 0; - SCReturnInt(-1); + SCReturnInt(0); } SCLogDebug("[5] ByteCount (%u/%u) ByteCount %u parsed %"PRIu64" input_len %u", sstate->bytesprocessed, NBSS_HDR_LEN + SMB_HDR_LEN + 3, @@ -1074,9 +1093,9 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, } else if (input_len) { SCLogDebug("Error parsing SMB Byte Count Data\n"); sstate->bytesprocessed = 0; - SCReturnInt(-1); + SCReturnInt(0); } - SCLogDebug("[6] Parsing ByteCount (%u/%u) ByteCount %u parsed %"PRIu64" input_len %u", + SCLogDebug("[6] Parsing ByteCount (%u/%u) ByteCount %u parsed %"PRIu64" input_len %u\n", sstate->bytesprocessed, NBSS_HDR_LEN + SMB_HDR_LEN + 1 + sstate->wordcount.wordcount + 2 + sstate->bytecount.bytecount, sstate->bytecount.bytecount, parsed, input_len); }