From c3934012127bbbfd73f14a7d9533ae0eb6b7175e Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Tue, 9 Mar 2021 15:54:16 +0100 Subject: [PATCH 1/3] enip: improve probing parser Strict length for register sessions NOP command must have options=0 --- src/app-layer-enip.c | 87 ++++++++++++++++++++++++++++++++++---------- 1 file changed, 67 insertions(+), 20 deletions(-) diff --git a/src/app-layer-enip.c b/src/app-layer-enip.c index 2db644a2bd01..a53b13dcd99d 100644 --- a/src/app-layer-enip.c +++ b/src/app-layer-enip.c @@ -359,7 +359,7 @@ static AppLayerResult ENIPParse(Flow *f, void *state, AppLayerParserState *pstat SCReturnStruct(APP_LAYER_OK); } - +#define ENIP_LEN_REGISTER_SESSION 4 // protocol u16, options u16 static uint16_t ENIPProbingParser(Flow *f, uint8_t direction, const uint8_t *input, uint32_t input_len, uint8_t *rdir) @@ -371,43 +371,90 @@ static uint16_t ENIPProbingParser(Flow *f, uint8_t direction, return ALPROTO_UNKNOWN; } uint16_t cmd; + uint16_t enip_len; uint32_t status; - int ret = ByteExtractUint16(&cmd, BYTE_LITTLE_ENDIAN, sizeof(uint16_t), - (const uint8_t *) (input)); + uint32_t option; + uint16_t nbitems; + + int ret = ByteExtractUint16( + &enip_len, BYTE_LITTLE_ENDIAN, sizeof(uint16_t), (const uint8_t *)(input + 2)); + if (ret < 0) { + return ALPROTO_FAILED; + } + if (enip_len < sizeof(ENIPEncapHdr)) { + return ALPROTO_FAILED; + } + ret = ByteExtractUint32( + &status, BYTE_LITTLE_ENDIAN, sizeof(uint32_t), (const uint8_t *)(input + 8)); + if (ret < 0) { + return ALPROTO_FAILED; + } + switch (status) { + case SUCCESS: + case INVALID_CMD: + case NO_RESOURCES: + case INCORRECT_DATA: + case INVALID_SESSION: + case INVALID_LENGTH: + case UNSUPPORTED_PROT_REV: + case ENCAP_HEADER_ERROR: + break; + default: + return ALPROTO_FAILED; + } + ret = ByteExtractUint16(&cmd, BYTE_LITTLE_ENDIAN, sizeof(uint16_t), (const uint8_t *)(input)); if(ret < 0) { return ALPROTO_FAILED; } + ret = ByteExtractUint32( + &option, BYTE_LITTLE_ENDIAN, sizeof(uint32_t), (const uint8_t *)(input + 20)); + if (ret < 0) { + return ALPROTO_FAILED; + } + //ok for all the known commands switch(cmd) { case NOP: - case LIST_SERVICES: - case LIST_IDENTITY: - case LIST_INTERFACES: + if (option != 0) { + return ALPROTO_FAILED; + } + break; case REGISTER_SESSION: + if (enip_len != ENIP_LEN_REGISTER_SESSION) { + return ALPROTO_FAILED; + } + break; case UNREGISTER_SESSION: + if (enip_len != ENIP_LEN_REGISTER_SESSION && enip_len != 0) { + // 0 for request and 4 for response + return ALPROTO_FAILED; + } + break; + case LIST_SERVICES: + case LIST_IDENTITY: case SEND_RR_DATA: case SEND_UNIT_DATA: case INDICATE_STATUS: case CANCEL: - ret = ByteExtractUint32(&status, BYTE_LITTLE_ENDIAN, - sizeof(uint32_t), - (const uint8_t *) (input + 8)); + break; + case LIST_INTERFACES: + if (input_len < sizeof(ENIPEncapHdr) + 2) { + SCLogDebug("length too small to be a ENIP LIST_INTERFACES"); + return ALPROTO_UNKNOWN; + } + ret = ByteExtractUint16( + &nbitems, BYTE_LITTLE_ENDIAN, sizeof(uint16_t), (const uint8_t *)(input)); if(ret < 0) { return ALPROTO_FAILED; } - switch(status) { - case SUCCESS: - case INVALID_CMD: - case NO_RESOURCES: - case INCORRECT_DATA: - case INVALID_SESSION: - case INVALID_LENGTH: - case UNSUPPORTED_PROT_REV: - case ENCAP_HEADER_ERROR: - return ALPROTO_ENIP; + if (enip_len < sizeof(ENIPEncapHdr) + 2 * (size_t)nbitems) { + return ALPROTO_FAILED; } + break; + default: + return ALPROTO_FAILED; } - return ALPROTO_FAILED; + return ALPROTO_ENIP; } /** From 344a6c30702a864539be65520348169b48e9adf6 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Tue, 9 Mar 2021 21:00:36 +0100 Subject: [PATCH 2/3] dns: improve probing parser Checks opcode is valid Checks additional_rr do not exceed message length Better logic for incomplete cases --- rust/src/dns/dns.rs | 47 +++++++++++++++++++++++++++++++++++---------- 1 file changed, 37 insertions(+), 10 deletions(-) diff --git a/rust/src/dns/dns.rs b/rust/src/dns/dns.rs index 350c165aee92..ce796127ae1b 100644 --- a/rust/src/dns/dns.rs +++ b/rust/src/dns/dns.rs @@ -659,23 +659,50 @@ impl DNSState { } } +fn probe_header_validity(header : DNSHeader, rlen: usize) -> (bool, bool, bool) { + let opcode = ((header.flags >> 11) & 0xf) as u8; + if opcode >= 7 { + //unassigned opcode + return (false, false, false); + } + if header.additional_rr as usize + + header.answer_rr as usize + + header.authority_rr as usize + + header.questions as usize + > rlen + { + //not enough data for additional records + return (false, false, false); + } + let is_request = header.flags & 0x8000 == 0; + return (true, is_request, false); +} + /// Probe input to see if it looks like DNS. -fn probe(input: &[u8]) -> (bool, bool) { - match parser::dns_parse_request(input) { +fn probe(input: &[u8], dlen: usize) -> (bool, bool, bool) { + let i2 = if input.len() <= dlen { input } else { &input[..dlen] }; + match parser::dns_parse_request(i2) { Ok((_, request)) => { - let is_request = request.header.flags & 0x8000 == 0; - return (true, is_request); + return probe_header_validity(request.header, dlen); }, - Err(_) => (false, false), + Err(nom::Err::Incomplete(_)) => { + match parser::dns_parse_header(input) { + Ok((_, header)) => { + return probe_header_validity(header, dlen); + } + Err(nom::Err::Incomplete(_)) => (false, false, true), + Err(_) => (false, false, false), + } + } + Err(_) => (false, false, false), } } /// Probe TCP input to see if it looks like DNS. pub fn probe_tcp(input: &[u8]) -> (bool, bool, bool) { - match be_u16(input) as IResult<&[u8],_> { - Ok((rem, _)) => { - let r = probe(rem); - return (r.0, r.1, false); + match be_u16(input) as IResult<&[u8],u16> { + Ok((rem, dlen)) => { + return probe(rem, dlen as usize); }, Err(nom::Err::Incomplete(_)) => { return (false, false, true); @@ -961,7 +988,7 @@ pub extern "C" fn rs_dns_probe( let slice: &[u8] = unsafe { std::slice::from_raw_parts(input as *mut u8, len as usize) }; - let (is_dns, is_request) = probe(slice); + let (is_dns, is_request, _) = probe(slice, slice.len()); if is_dns { let dir = if is_request { core::STREAM_TOSERVER From f355941cf2bcb3d6c8942b39aa30e62bab8c1392 Mon Sep 17 00:00:00 2001 From: Philippe Antoine Date: Tue, 16 Mar 2021 13:07:16 +0100 Subject: [PATCH 3/3] nfs: improve probing parser Checks credentials flavor is known --- rust/src/nfs/nfs.rs | 2 +- rust/src/nfs/types.rs | 8 ++++++++ 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/rust/src/nfs/nfs.rs b/rust/src/nfs/nfs.rs index 5869ca770e6f..8bcd1e5c45d0 100644 --- a/rust/src/nfs/nfs.rs +++ b/rust/src/nfs/nfs.rs @@ -1739,7 +1739,7 @@ pub fn nfs_probe(i: &[u8], direction: u8) -> i8 { rpc.program == 100003 && rpc.procedure <= NFSPROC3_COMMIT { - return 1; + return rpc_auth_type_known(rpc.creds_flavor); } else { return -1; } diff --git a/rust/src/nfs/types.rs b/rust/src/nfs/types.rs index d3f313a39d0b..c8ddfafc11a2 100644 --- a/rust/src/nfs/types.rs +++ b/rust/src/nfs/types.rs @@ -177,6 +177,14 @@ pub fn rpc_auth_type_string(auth_type: u32) -> String { }.to_string() } +pub fn rpc_auth_type_known(auth_type: u32) -> i8 { + // RPCAUTH_GSS is the maximum + if auth_type <= RPCAUTH_GSS { + return 1; + } + return -1; +} + /* http://www.iana.org/assignments/rpc-authentication-numbers/rpc-authentication-numbers.xhtml */ pub const RPCAUTH_OK: u32 = 0; // success/failed at remote end [RFC5531] pub const RPCAUTH_BADCRED: u32 = 1; // bad credential (seal broken) [RFC5531]