From 11298a5b2cb8c2f4933618e7eb9f5f72b9419a87 Mon Sep 17 00:00:00 2001 From: John Thacker Date: Fri, 22 Oct 2021 23:07:57 -0400 Subject: DCERPC: Strengthen heuristic, fix PDU length The fragment length field of DCERPC connection-oriented PDUs includes the length of the fixed header, and so must be at least that large. Don't return a (bogus) PDU length zero from get_dcerpc_pdu_len, because tcp_dissect_pdus interprets that as "need one more segment" instead of as a bogus value; instead return one, which the TCP dissector will correctly recognize as bogus. Also, take into account the offset passed into get_dcerpc_pdu_len (it is almost always 0, which is why the code previously worked), and increase the fixed length value passed to tcp_dissect_pdus to the real fixed header length (so that the TCP dissector will recognize more bogus values as bogus.) Fix #14728. --- epan/dissectors/packet-dcerpc.c | 28 ++++++++++++++++++++++------ 1 file changed, 22 insertions(+), 6 deletions(-) (limited to 'epan/dissectors/packet-dcerpc.c') diff --git a/epan/dissectors/packet-dcerpc.c b/epan/dissectors/packet-dcerpc.c index 862cb2dd77..67f147dca8 100644 --- a/epan/dissectors/packet-dcerpc.c +++ b/epan/dissectors/packet-dcerpc.c @@ -5438,6 +5438,7 @@ dissect_dcerpc_cn_rts(tvbuff_t *tvb, gint offset, packet_info *pinfo, } } +/* Test to see if this looks like a connection oriented PDU */ static gboolean is_dcerpc(tvbuff_t *tvb, int offset, packet_info *pinfo _U_) { @@ -5445,6 +5446,7 @@ is_dcerpc(tvbuff_t *tvb, int offset, packet_info *pinfo _U_) guint8 rpc_ver_minor; guint8 ptype; guint8 drep[4]; + guint16 frag_len; if (!tvb_bytes_exist(tvb, offset, sizeof(e_dce_cn_common_hdr_t))) return FALSE; /* not enough information to check */ @@ -5466,6 +5468,11 @@ is_dcerpc(tvbuff_t *tvb, int offset, packet_info *pinfo _U_) return FALSE; if (drep[1] > DCE_RPC_DREP_FP_IBM) return FALSE; + offset += (int)sizeof(drep); + frag_len = dcerpc_tvb_get_ntohs(tvb, offset, drep); + if (frag_len < sizeof(e_dce_cn_common_hdr_t)) { + return FALSE; + } return TRUE; } @@ -5861,15 +5868,24 @@ dissect_dcerpc_cn_bs(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void * static guint get_dcerpc_pdu_len(packet_info *pinfo _U_, tvbuff_t *tvb, - int offset _U_, void *data _U_) + int offset, void *data _U_) { guint8 drep[4]; guint16 frag_len; - /* XXX: why does htis not take offset into account? */ - tvb_memcpy(tvb, (guint8 *)drep, 4, sizeof(drep)); - frag_len = dcerpc_tvb_get_ntohs(tvb, 8, drep); + tvb_memcpy(tvb, (guint8 *)drep, offset+4, sizeof(drep)); + frag_len = dcerpc_tvb_get_ntohs(tvb, offset+8, drep); + if (!frag_len) { + /* tcp_dissect_pdus() interprets a 0 return value as meaning + * "a PDU starts here, but the length cannot be determined yet, so + * we need at least one more segment." However, a frag_len of 0 here + * is instead a bogus length. Instead return 1, another bogus length + * also less than our fixed length, so that the TCP dissector will + * correctly interpret it as a bogus and report an error. + */ + frag_len = 1; + } return frag_len; } @@ -5895,7 +5911,7 @@ dissect_dcerpc_tcp_heur(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, voi decode_data = dcerpc_get_decode_data(pinfo); decode_data->dcetransporttype = DCE_TRANSPORT_UNKNOWN; - tcp_dissect_pdus(tvb, pinfo, tree, dcerpc_cn_desegment, 10, get_dcerpc_pdu_len, dissect_dcerpc_pdu, data); + tcp_dissect_pdus(tvb, pinfo, tree, dcerpc_cn_desegment, sizeof(e_dce_cn_common_hdr_t), get_dcerpc_pdu_len, dissect_dcerpc_pdu, data); return TRUE; } @@ -5907,7 +5923,7 @@ dissect_dcerpc_tcp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *da decode_data = dcerpc_get_decode_data(pinfo); decode_data->dcetransporttype = DCE_TRANSPORT_UNKNOWN; - tcp_dissect_pdus(tvb, pinfo, tree, dcerpc_cn_desegment, 10, get_dcerpc_pdu_len, dissect_dcerpc_pdu, data); + tcp_dissect_pdus(tvb, pinfo, tree, dcerpc_cn_desegment, sizeof(e_dce_cn_common_hdr_t), get_dcerpc_pdu_len, dissect_dcerpc_pdu, data); return tvb_captured_length(tvb); } -- cgit v1.2.3