From 35c400a3426a8c1ffa2e79a83f7c0ad4f4848bc7 Mon Sep 17 00:00:00 2001 From: Kirby Kuehl Date: Thu, 11 Feb 2010 10:32:09 -0600 Subject: [PATCH 1/3] bug 88 validate dcerpc header --- src/app-layer-dcerpc-common.h | 2 +- src/app-layer-dcerpc.c | 50 ++++++++++++++++++--------- src/app-layer-nbss.h | 2 +- src/app-layer-smb.c | 76 +++++++++++++++++++++++----------------- src/app-layer-smb.h | 6 ++-- 5 files changed, 83 insertions(+), 53 deletions(-) diff --git a/src/app-layer-dcerpc-common.h b/src/app-layer-dcerpc-common.h index e5bcc0d..f9a24b4 100644 --- a/src/app-layer-dcerpc-common.h +++ b/src/app-layer-dcerpc-common.h @@ -169,7 +169,7 @@ typedef struct DCERPC_ { } p_cont_elem_t; */ -uint32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len); +int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len); void hexdump(const void *buf, size_t len); void printUUID(char *type, struct uuid_entry *uuid); diff --git a/src/app-layer-dcerpc.c b/src/app-layer-dcerpc.c index a427848..ccc38f7 100644 --- a/src/app-layer-dcerpc.c +++ b/src/app-layer-dcerpc.c @@ -858,18 +858,24 @@ static uint32_t StubDataParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_le * A fast path for normal decoding is used when there is enough bytes * present to parse the entire header. A slow path is used to parse * fragmented packets. + * \retval -1 if DCEPRC Header does not validate + * \retval Number of bytes processed */ -static uint32_t DCERPCParseHeader(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { +static int DCERPCParseHeader(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { SCEnter(); uint8_t *p = input; if (input_len) { switch (dcerpc->bytesprocessed) { case 0: if (input_len >= DCERPC_HDR_LEN) { - //if (*p != 5) SCReturnUInt(); - //if (!(*(p + 1 ) == 0 || (*(p + 1) == 1))) SCReturnInt(0); dcerpc->dcerpchdr.rpc_vers = *p; dcerpc->dcerpchdr.rpc_vers_minor = *(p + 1); + 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); + } dcerpc->dcerpchdr.type = *(p + 2); dcerpc->dcerpchdr.pfc_flags = *(p + 3); dcerpc->dcerpchdr.packed_drep[0] = *(p + 4); @@ -896,18 +902,21 @@ static uint32_t DCERPCParseHeader(DCERPC *dcerpc, uint8_t *input, uint32_t input dcerpc->dcerpchdr.call_id |= *(p + 15) << 24; } dcerpc->bytesprocessed = DCERPC_HDR_LEN; - SCReturnUInt(16U); + SCReturnInt(16); break; } else { dcerpc->dcerpchdr.rpc_vers = *(p++); - // if dcerpc->dcerpchdr.rpc_vers != 5) SCReturnInt(2); if (!(--input_len)) break; } case 1: dcerpc->dcerpchdr.rpc_vers_minor = *(p++); - // if ((sdcerpc->dcerpchdr.rpc_vers_minor != 0) || - // (dcerpc->dcerpchdr.rpc_vers_minor != 1)) SCReturnInt(3); + 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); + } if (!(--input_len)) break; case 2: @@ -974,17 +983,24 @@ static uint32_t DCERPCParseHeader(DCERPC *dcerpc, uint8_t *input, uint32_t input } } dcerpc->bytesprocessed += (p - input); - SCReturnUInt((uint32_t)(p - input)); + SCReturnInt((p - input)); } -uint32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { +int32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { SCEnter(); uint32_t retval = 0; uint32_t parsed = 0; + int hdrretval = 0; + while (dcerpc->bytesprocessed < DCERPC_HDR_LEN && input_len) { - retval = DCERPCParseHeader(dcerpc, input, input_len); - parsed += retval; - input_len -= retval; + hdrretval = DCERPCParseHeader(dcerpc, input, input_len); + if (hdrretval == -1) { + dcerpc->bytesprocessed = 0; + SCReturnInt(-1); + } else { + parsed += hdrretval; + input_len -= hdrretval; + } } SCLogDebug("Done with DCERPCParseHeader bytesprocessed %u/%u left %u\n", dcerpc->bytesprocessed, dcerpc->dcerpchdr.frag_length, input_len); @@ -1198,18 +1214,20 @@ uint32_t DCERPCParser(DCERPC *dcerpc, uint8_t *input, uint32_t input_len) { dcerpc->bytesprocessed = 0; break; } - SCReturnUInt(parsed); + SCReturnInt(parsed); } static int DCERPCParse(Flow *f, void *dcerpc_state, AppLayerParserState *pstate, uint8_t *input, uint32_t input_len, AppLayerParserResult *output) { SCEnter(); - + int32_t retval = 0; DCERPCState *sstate = (DCERPCState *) dcerpc_state; - DCERPCParser(&sstate->dcerpc, input, input_len); - + retval = DCERPCParser(&sstate->dcerpc, input, input_len); + if (retval == -1) { + SCReturnInt(-1); + } if (pstate == NULL) SCReturnInt(-1); diff --git a/src/app-layer-nbss.h b/src/app-layer-nbss.h index 0cb24fe..42a8ff1 100644 --- a/src/app-layer-nbss.h +++ b/src/app-layer-nbss.h @@ -41,6 +41,6 @@ typedef struct nbss_hdr_ { uint8_t flags; uint32_t length; }NBSSHdr; -#define NBSS_HDR_LEN 4U +#define NBSS_HDR_LEN 4 #endif /* APPLAYERNBSS_H_ */ diff --git a/src/app-layer-smb.c b/src/app-layer-smb.c index a0b34c3..4c6fea5 100644 --- a/src/app-layer-smb.c +++ b/src/app-layer-smb.c @@ -508,17 +508,17 @@ 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 - NBSS_HDR_LEN) > sstate->andx.dataoffset) { + if ((uint64_t)(sstate->bytesprocessed - NBSS_HDR_LEN) > sstate->andx.dataoffset) { sstate->andx.paddingparsed = 1; SCReturnUInt((uint32_t)(p - input)); } - while (((sstate->bytesprocessed - NBSS_HDR_LEN) + (p - input)) + while (((uint64_t)(sstate->bytesprocessed - NBSS_HDR_LEN) + (p - input)) < sstate->andx.dataoffset && sstate->bytecount.bytecountleft-- && input_len--) { SCLogDebug("0x%02x ", *p); p++; } - if (((sstate->bytesprocessed - NBSS_HDR_LEN) + (p - input)) + if (((uint64_t)(sstate->bytesprocessed - NBSS_HDR_LEN) + (p - input)) == sstate->andx.dataoffset) { sstate->andx.paddingparsed = 1; } @@ -528,20 +528,25 @@ static uint32_t PaddingParser(void *smb_state, AppLayerParserState *pstate, /** * \brief Parse WriteAndX and ReadAndX Data - * \todo Hand off to DCERPC parser for DCERPC over SMB + * \retval -1 f DCERPCParser does not validate + * \retval Number of bytes processed */ -static uint32_t DataParser(void *smb_state, AppLayerParserState *pstate, +static int32_t DataParser(void *smb_state, AppLayerParserState *pstate, uint8_t *input, uint32_t input_len, AppLayerParserResult *output) { SCEnter(); SMBState *sstate = (SMBState *) smb_state; - uint32_t parsed = 0; + int32_t parsed = 0; if (sstate->andx.paddingparsed) { - parsed = DCERPCParser(&sstate->dcerpc, input, input_len); - sstate->bytesprocessed += parsed; - sstate->bytecount.bytecountleft -= parsed; - input_len -= parsed; + parsed = DCERPCParser(&sstate->dcerpc, input, input_len); + if (parsed == -1) { + SCReturnInt(-1); + } else { + sstate->bytesprocessed += parsed; + sstate->bytecount.bytecountleft -= parsed; + input_len -= parsed; + } } - SCReturnUInt(parsed); + SCReturnInt(parsed); } /** @@ -655,7 +660,7 @@ static uint32_t SMBParseByteCount(Flow *f, void *smb_state, SCEnter(); SMBState *sstate = (SMBState *) smb_state; uint8_t *p = input; - uint32_t retval = 0; + int32_t retval = 0; uint32_t parsed = 0; if (((sstate->smb.flags & SMB_FLAGS_SERVER_TO_REDIR) && sstate->smb.command == SMB_COM_READ_ANDX) || (((sstate->smb.flags @@ -672,8 +677,19 @@ static uint32_t SMBParseByteCount(Flow *f, void *smb_state, if (sstate->andx.datalength && input_len) { retval = DataParser(sstate, pstate, input + parsed, input_len, output); - parsed += retval; - input_len -= retval; + if (retval != -1) { + parsed += retval; + input_len -= retval; + } else { /* Did not Validate as DCERPC over SMB */ + while (sstate->bytecount.bytecountleft-- && input_len--) { + SCLogDebug("0x%02x bytecount %u/%u input_len %u", *p, + sstate->bytecount.bytecountleft, + sstate->bytecount.bytecount, input_len); + p++; + } + sstate->bytesprocessed += (p - input); + SCReturnUInt((p - input)); + } } SCReturnUInt(retval); } @@ -685,7 +701,7 @@ static uint32_t SMBParseByteCount(Flow *f, void *smb_state, p++; } sstate->bytesprocessed += (p - input); - SCReturnUInt((uint32_t)(p - input)); + SCReturnUInt((p - input)); } static uint32_t NBSSParseHeader(Flow *f, void *smb_state, @@ -733,7 +749,7 @@ static uint32_t NBSSParseHeader(Flow *f, void *smb_state, /** * \brief SMBParseHeader parses and validates the 32 byte SMB Header */ -static uint32_t SMBParseHeader(Flow *f, void *smb_state, +static int SMBParseHeader(Flow *f, void *smb_state, AppLayerParserState *pstate, uint8_t *input, uint32_t input_len, AppLayerParserResult *output) { SCEnter(); @@ -745,7 +761,7 @@ static uint32_t SMBParseHeader(Flow *f, void *smb_state, if (input_len >= SMB_HDR_LEN) { if (memcmp(p, "\xff\x53\x4d\x42", 4) != 0) { SCLogDebug("SMB Header did not validate"); - SCReturnUInt(0); + SCReturnInt(-1); } sstate->smb.command = *(p + 4); sstate->smb.status = *(p + 5) << 24; @@ -774,31 +790,27 @@ static uint32_t SMBParseHeader(Flow *f, void *smb_state, sstate->smb.mid = *(p + 30) << 8; sstate->smb.mid |= *(p + 31); sstate->bytesprocessed += SMB_HDR_LEN; - SCReturnUInt(32U); + SCReturnInt(32); break; } else { - //sstate->smb.protocol[0] = *(p++); if (*(p++) != 0xff) - SCReturnInt(0); + SCReturnInt(-1); if (!(--input_len)) break; } case 5: - //sstate->smb.protocol[1] = *(p++); if (*(p++) != 'S') - SCReturnInt(0); + SCReturnInt(-1); if (!(--input_len)) break; case 6: - //sstate->smb.protocol[2] = *(p++); if (*(p++) != 'M') - SCReturnInt(0); + SCReturnInt(-1); if (!(--input_len)) break; case 7: - //sstate->smb.protocol[3] = *(p++); if (*(p++) != 'B') - SCReturnInt(0); + SCReturnInt(-1); if (!(--input_len)) break; case 8: @@ -916,7 +928,7 @@ static uint32_t SMBParseHeader(Flow *f, void *smb_state, } } sstate->bytesprocessed += (p - input); - SCReturnUInt((uint32_t)(p - input)); + SCReturnInt((p - input)); } static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, @@ -952,17 +964,17 @@ static int SMBParse(Flow *f, void *smb_state, AppLayerParserState *pstate, && sstate->bytesprocessed < NBSS_HDR_LEN + SMB_HDR_LEN)) { retval = SMBParseHeader(f, smb_state, pstate, input + parsed, input_len, output); - if (retval) { + if (retval == -1) { + SCLogDebug("Error parsing SMB Header\n"); + sstate->bytesprocessed = 0; + SCReturnInt(1); + } else { parsed += retval; input_len -= retval; SCLogDebug( "SMB Header (%u/%u) Command 0x%02x parsed %ld input_len %u", sstate->bytesprocessed, NBSS_HDR_LEN + SMB_HDR_LEN, sstate->smb.command, parsed, input_len); - } else if (input_len) { - SCLogDebug("Error parsing SMB Word Count\n"); - parsed += input_len; - input_len = 0; } } diff --git a/src/app-layer-smb.h b/src/app-layer-smb.h index 4737e55..df624b9 100644 --- a/src/app-layer-smb.h +++ b/src/app-layer-smb.h @@ -29,9 +29,9 @@ typedef struct smb_hdr_ { uint16_t uid; uint16_t mid; }SMBHdr; -#define SMB_HDR_LEN 32U -#define MINIMUM_SMB_LEN 35U -#define NBSS_SMB_HDRS_LEN 36U +#define SMB_HDR_LEN 32 +#define MINIMUM_SMB_LEN 35 +#define NBSS_SMB_HDRS_LEN 36 typedef struct wordcount_ { uint8_t wordcount; -- 1.6.6