From cad248ce3bf53026cc837fedeaca65d0f20ea3b5 Mon Sep 17 00:00:00 2001 From: Gerald Combs Date: Tue, 8 Oct 2024 11:56:28 -0700 Subject: [PATCH] AppleTalk: Make sure we have valid addresses Make sure ATP, ZIP, and ASP have valid addresses. Use sizeof instead of a hard-coded value in a few places. Fixes #20114 (cherry picked from commit 3de741321f85c205c0a8266c40f33cb0013bd1d2) Conflicts: epan/dissectors/packet-atalk.c epan/dissectors/packet-reload-framing.c --- epan/dissectors/packet-atalk.c | 44 ++++++++++++++++++++++++---------- 1 file changed, 32 insertions(+), 12 deletions(-) epan/dissectors/packet-reload-framing.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/epan/dissectors/packet-atalk.c b/epan/dissectors/packet-atalk.c index 396e7af5194..065d6aedb68 100644 --- a/epan/dissectors/packet-atalk.c +++ b/epan/dissectors/packet-atalk.c @@ -232,9 +232,18 @@ static int hf_asp_attn_code = -1; static int hf_asp_seq = -1; static int hf_asp_size = -1; +/* + * Structure used to represent a DDP address; gives the layout of the + * data pointed to by an Appletalk "address" structure. + */ +struct atalk_ddp_addr { + guint16 net; + guint8 node; +}; + typedef struct { guint32 conversation; - guint8 src[4]; + guint8 src[sizeof(struct atalk_ddp_addr)]; guint16 tid; } asp_request_key; @@ -502,6 +511,10 @@ static const value_string asp_error_vals[] = { {0, NULL } }; value_string_ext asp_error_vals_ext = VALUE_STRING_EXT_INIT(asp_error_vals); +static bool is_ddp_address(address *addr) { + return addr->type == atalk_address_type && addr->len == sizeof(struct atalk_ddp_addr); +} + /* * hf_index must be a FT_UINT_STRING type * Are these always in a Mac extended character set? Should we have a @@ -744,6 +757,12 @@ dissect_atp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_) conversation_t *conversation; asp_request_val *request_val = NULL; + // ATP is carried over DDP + if (!(is_ddp_address(&pinfo->src) && is_ddp_address(&pinfo->dst))) { + return 0; + } + + col_set_str(pinfo->cinfo, COL_PROTOCOL, "ATP"); ctrlinfo = tvb_get_guint8(tvb, offset); @@ -770,7 +789,7 @@ dissect_atp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _U_) asp_request_key request_key; request_key.conversation = conversation->conv_index; - memcpy(request_key.src, (!atp_asp_dsi_info.reply)?pinfo->src.data:pinfo->dst.data, 4); + memcpy(request_key.src, (!atp_asp_dsi_info.reply)?pinfo->src.data:pinfo->dst.data, sizeof(struct atalk_ddp_addr)); request_key.tid = atp_asp_dsi_info.tid; request_val = (asp_request_val *) wmem_map_lookup(atp_request_hash, &request_key); @@ -1018,7 +1037,7 @@ get_transaction(tvbuff_t *tvb, packet_info *pinfo, struct atp_asp_dsi_info *atp_ conversation = find_or_create_conversation(pinfo); request_key.conversation = conversation->conv_index; - memcpy(request_key.src, (!atp_asp_dsi_info->reply)?pinfo->src.data:pinfo->dst.data, 4); + memcpy(request_key.src, (!atp_asp_dsi_info->reply)?pinfo->src.data:pinfo->dst.data, sizeof(struct atalk_ddp_addr)); request_key.tid = atp_asp_dsi_info->tid; request_val = (asp_request_val *) wmem_map_lookup(asp_request_hash, &request_key); @@ -1051,6 +1070,11 @@ dissect_asp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data) if (data == NULL) return 0; + // ASP is carried over ATP/DDP + if (!(is_ddp_address(&pinfo->src) && is_ddp_address(&pinfo->dst))) { + return 0; + } + col_set_str(pinfo->cinfo, COL_PROTOCOL, "ASP"); col_clear(pinfo->cinfo, COL_INFO); @@ -1183,15 +1207,6 @@ dissect_asp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data) /* ----------------------------- ZIP protocol cf. inside appletalk chap. 8 */ -/* - * Structure used to represent a DDP address; gives the layout of the - * data pointed to by an Appletalk "address" structure. - */ -struct atalk_ddp_addr { - guint16 net; - guint8 node; -}; - static int atalk_str_len(const address* addr _U_) { @@ -1241,6 +1256,11 @@ dissect_atp_zip(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data) if (data == NULL) return 0; + // ATP ZIP is carried over DDP + if (!(is_ddp_address(&pinfo->src) && is_ddp_address(&pinfo->dst))) { + return 0; + } + col_set_str(pinfo->cinfo, COL_PROTOCOL, "ZIP"); col_clear(pinfo->cinfo, COL_INFO); diff --git a/epan/dissectors/packet-reload-framing.c b/epan/dissectors/packet-reload-framing.c index eac1744c315..688068bbe61 100644 --- a/epan/dissectors/packet-reload-framing.c +++ b/epan/dissectors/packet-reload-framing.c @@ -124,8 +124,14 @@ dissect_reload_framing_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tr effective_length = tvb_captured_length(tvb); /* First, make sure we have enough data to do the check. */ - if (effective_length < MIN_HDR_LENGTH) + if (effective_length < MIN_HDR_LENGTH) { return 0; + } + + /* Next, make sure we can create transaction ID keys. */ + if (!(pinfo->src.data && pinfo->dst.data)) { + return 0; + } conversation = find_conversation_pinfo(pinfo, 0); if (conversation) @@ -194,14 +200,14 @@ dissect_reload_framing_message(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tr transaction_id_key[1].length = 1; transaction_id_key[1].key = &pinfo->srcport; transaction_id_key[2].length = (pinfo->src.len) / (guint)sizeof(guint32); - transaction_id_key[2].key = (guint32 *)g_malloc(pinfo->src.len); + transaction_id_key[2].key = (guint32 *)wmem_alloc(wmem_file_scope(), pinfo->src.len); memcpy(transaction_id_key[2].key, pinfo->src.data, pinfo->src.len); } else { transaction_id_key[1].length = 1; transaction_id_key[1].key = &pinfo->destport; transaction_id_key[2].length = (pinfo->dst.len) / (guint)sizeof(guint32); - transaction_id_key[2].key = (guint32 *)g_malloc(pinfo->dst.len); + transaction_id_key[2].key = (guint32 *)wmem_alloc(wmem_file_scope(), pinfo->dst.len); memcpy(transaction_id_key[2].key, pinfo->dst.data, pinfo->dst.len); } transaction_id_key[3].length=0;