From dc04b00bae450b614891e30a8e64d503dc397611 Mon Sep 17 00:00:00 2001 From: John Thacker Date: Sun, 29 Mar 2026 17:51:26 -0400 Subject: [PATCH] BEEP: Prevent overflow and/or stack overflow Just throw a reported bounds exception if the size is an illegal value rather than trying to figure out what it can be set to that will avoid an infinite loop. Most of the values from dissect_beep_int are not used, so allow the value to be NULL. All the values are actually unsigned; some are in the range 0...INT32_MAX and one is 0...UINT32_MAX. Also add a recursion check in the other places where recursion happens, but we shouldn't really need one there. (It was an overflow issue really.) Fix #21120 AI-Assisted: no (backported from commit 403b9d269f827e772e5fe1ac8433db723d8b7010) --- epan/dissectors/packet-beep.c | 45 +++++++++++++++++++++-------------- 1 file changed, 27 insertions(+), 18 deletions(-) Index: wireshark-3.6.24/epan/dissectors/packet-beep.c =================================================================== --- wireshark-3.6.24.orig/epan/dissectors/packet-beep.c +++ wireshark-3.6.24/epan/dissectors/packet-beep.c @@ -18,10 +18,12 @@ #include #include +#include #include #include #include #include +#include #include #if defined(DEBUG_BEEP_HASH) #include @@ -358,13 +360,15 @@ dissect_beep_mime_header(tvbuff_t *tvb, static int dissect_beep_int(tvbuff_t *tvb, packet_info *pinfo, int offset, - proto_tree *tree, int hf, int *val, int *hfa[]) + proto_tree *tree, int hf, unsigned *val, int *hfa[]) { proto_item *hidden_item; - int ival, ind = 0; + unsigned ival, ind = 0; unsigned int len = num_len(tvb, offset); - ival = (int)strtol(tvb_get_string_enc(pinfo->pool, tvb, offset, len, ENC_ASCII), NULL, 10); + /* XXX - Should check for conversion errors and, for values except for + * seqno, values outside the range. */ + ival = (unsigned)strtoul((char*)tvb_get_string_enc(pinfo->pool, tvb, offset, len, ENC_ASCII), NULL, 10); proto_tree_add_uint(tree, hf, tvb, offset, len, ival); while (hfa[ind]) { @@ -375,7 +379,9 @@ dissect_beep_int(tvbuff_t *tvb, packet_i } - *val = ival; /* Return the value */ + if (val) { + *val = ival; /* Return the value */ + } return len; @@ -443,8 +449,8 @@ dissect_beep_tree(tvbuff_t *tvb, int off { proto_tree *ti = NULL, *hdr = NULL; /*proto_item *hidden_item;*/ - int st_offset, msgno, ansno, seqno, size, channel, ackno, window, cc, - more; + int st_offset, cc, more; + unsigned size; const char * cmd_temp = NULL; int is_ANS = 0; @@ -479,11 +485,11 @@ dissect_beep_tree(tvbuff_t *tvb, int off offset += 4; /* Get the channel */ - offset += dissect_beep_int(tvb, pinfo, offset, hdr, hf_beep_channel, &channel, req_chan_hfa); + offset += dissect_beep_int(tvb, pinfo, offset, hdr, hf_beep_channel, NULL, req_chan_hfa); offset += 1; /* Skip the space */ /* Dissect the message number */ - offset += dissect_beep_int(tvb, pinfo, offset, hdr, hf_beep_msgno, &msgno, req_msgno_hfa); + offset += dissect_beep_int(tvb, pinfo, offset, hdr, hf_beep_msgno, NULL, req_msgno_hfa); offset += 1; /* skip the space */ /* Insert the more elements ... */ @@ -505,22 +511,24 @@ dissect_beep_tree(tvbuff_t *tvb, int off offset += 2; /* Skip the flag and the space ... */ /* now for the seqno */ - offset += dissect_beep_int(tvb, pinfo, offset, hdr, hf_beep_seqno, &seqno, req_seqno_hfa); + offset += dissect_beep_int(tvb, pinfo, offset, hdr, hf_beep_seqno, NULL, req_seqno_hfa); offset += 1; /* skip the space */ offset += dissect_beep_int(tvb, pinfo, offset, hdr, hf_beep_size, &size, req_size_hfa); - if (request_val) /* FIXME, is this the right order ... */ - request_val -> size = size; /* Stash this away */ - else if (beep_frame_data) { + if (size > INT32_MAX) { + THROW(ReportedBoundsError); + } + if (request_val) { /* FIXME, is this the right order ... */ + request_val->size = size; /* Stash this away */ + } else if (beep_frame_data) { beep_frame_data->pl_size = size; - if (beep_frame_data->pl_size < 0) beep_frame_data->pl_size = 0; /* FIXME: OK? */ } /* offset += 1; skip the space */ if (is_ANS) { /* We need to put in the ansno */ offset += 1; /* skip the space */ /* Dissect the message number */ - offset += dissect_beep_int(tvb, pinfo, offset, hdr, hf_beep_ansno, &ansno, req_ansno_hfa); + offset += dissect_beep_int(tvb, pinfo, offset, hdr, hf_beep_ansno, NULL, req_ansno_hfa); } if ((cc = check_term(tvb, pinfo, offset, hdr)) <= 0) { @@ -547,7 +555,7 @@ dissect_beep_tree(tvbuff_t *tvb, int off if (tvb_reported_length_remaining(tvb, offset) > 0) { /* Dissect what is left as payload */ - int pl_size = MIN(size, tvb_reported_length_remaining(tvb, offset)); + int pl_size = MIN((int)size, tvb_reported_length_remaining(tvb, offset)); /* Except, check the payload length, and only dissect that much */ @@ -593,19 +601,19 @@ dissect_beep_tree(tvbuff_t *tvb, int off offset += 1; - offset += dissect_beep_int(tvb, pinfo, offset, tree, hf_beep_channel, &channel, seq_chan_hfa); + offset += dissect_beep_int(tvb, pinfo, offset, tree, hf_beep_channel, NULL, seq_chan_hfa); /* Check the space: FIXME */ offset += 1; - offset += dissect_beep_int(tvb, pinfo, offset, tree, hf_beep_ackno, &ackno, seq_ackno_hfa); + offset += dissect_beep_int(tvb, pinfo, offset, tree, hf_beep_ackno, NULL, seq_ackno_hfa); /* Check the space: FIXME */ offset += 1; - offset += dissect_beep_int(tvb, pinfo, offset, tree, hf_beep_window, &window, seq_window_hfa); + offset += dissect_beep_int(tvb, pinfo, offset, tree, hf_beep_window, NULL, seq_window_hfa); if ((cc = check_term(tvb, pinfo, offset, tree)) <= 0) { @@ -703,7 +711,9 @@ dissect_beep_tree(tvbuff_t *tvb, int off } if (tvb_reported_length_remaining(tvb, offset) > 0) { + increment_dissection_depth(pinfo); offset += dissect_beep_tree(tvb, offset, pinfo, tree, request_val, beep_frame_data); + decrement_dissection_depth(pinfo); } }