From 6f56fc9496db158218243ea87e3660c874a0bab0 Mon Sep 17 00:00:00 2001 From: Gerald Combs Date: Mon, 6 Apr 2020 10:15:31 -0700 Subject: [PATCH] BACapp: Add a nesting / recursion check. Track our recursion depth in fAbstractSyntaxNType. It calls several functions which in turn call it, which makes it easy to overflow the stack. Bug: 16474 Change-Id: Ibad29272f99449bfa13b7422692e20ba8a79e19c Reviewed-on: https://code.wireshark.org/review/36725 Reviewed-by: Gerald Combs Petri-Dish: Gerald Combs Tested-by: Petri Dish Buildbot Reviewed-by: Anders Broman (cherry picked from commit 15dc2f6bd4c9a674333cbc97260362524d5364de) Reviewed-on: https://code.wireshark.org/review/36736 --- epan/dissectors/packet-bacapp.c | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) diff --git a/epan/dissectors/packet-bacapp.c b/epan/dissectors/packet-bacapp.c index 133783b..308da49 100644 --- a/epan/dissectors/packet-bacapp.c +++ b/epan/dissectors/packet-bacapp.c @@ -19,6 +19,7 @@ #include #include #include +#include #include #include "packet-bacapp.h" @@ -43,6 +44,7 @@ static int bacapp_tap = -1; #define BACAPP_SEGMENT_NAK 0x02 #define BACAPP_SENT_BY 0x01 +#define BACAPP_MAX_RECURSION_DEPTH 100 // Arbitrary /** * dissect_bacapp ::= CHOICE { @@ -5862,6 +5864,7 @@ static gint ett_bacapp_value = -1; static expert_field ei_bacapp_bad_length = EI_INIT; static expert_field ei_bacapp_bad_tag = EI_INIT; static expert_field ei_bacapp_opening_tag = EI_INIT; +static expert_field ei_bacapp_max_recursion_depth_reached = EI_INIT; static gint32 propertyIdentifier = -1; static gint32 propertyArrayIndex = -1; @@ -7849,6 +7852,14 @@ fAbstractSyntaxNType(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, guint } else { g_snprintf(ar, sizeof(ar), "Abstract Type: "); } + + unsigned recursion_depth = GPOINTER_TO_UINT(p_get_proto_data(pinfo->pool, pinfo, proto_bacapp, 0)); + if (++recursion_depth >= BACAPP_MAX_RECURSION_DEPTH) { + proto_tree_add_expert(tree, pinfo, &ei_bacapp_max_recursion_depth_reached, tvb, 0, 0); + return offset; + } + p_add_proto_data(pinfo->pool, pinfo, proto_bacapp, 0, GUINT_TO_POINTER(recursion_depth)); + while (tvb_reported_length_remaining(tvb, offset) > 0) { /* exit loop if nothing happens inside */ lastoffset = offset; fTagHeader(tvb, pinfo, offset, &tag_no, &tag_info, &lvt); @@ -8502,6 +8513,9 @@ fAbstractSyntaxNType(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, guint } if (offset <= lastoffset) break; /* nothing happened, exit loop */ } + recursion_depth = GPOINTER_TO_UINT(p_get_proto_data(pinfo->pool, pinfo, proto_bacapp, 0)); + recursion_depth--; + p_add_proto_data(pinfo->pool, pinfo, proto_bacapp, 0, GUINT_TO_POINTER(recursion_depth)); return offset; } @@ -13974,6 +13988,9 @@ dissect_bacapp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void* data _ bacinfo.instance_ident = NULL; bacinfo.object_ident = NULL; + /* Recursion depth */ + p_add_proto_data(pinfo->pool, pinfo, proto_bacapp, 0, GUINT_TO_POINTER(0)); + switch (bacapp_type) { case BACAPP_TYPE_CONFIRMED_SERVICE_REQUEST: /* segmented messages have 2 additional bytes */ @@ -14426,6 +14443,8 @@ proto_register_bacapp(void) { &ei_bacapp_bad_length, { "bacapp.bad_length", PI_MALFORMED, PI_ERROR, "Wrong length indicated", EXPFILL }}, { &ei_bacapp_bad_tag, { "bacapp.bad_tag", PI_MALFORMED, PI_ERROR, "Wrong tag found", EXPFILL }}, { &ei_bacapp_opening_tag, { "bacapp.bad_opening_tag", PI_MALFORMED, PI_ERROR, "Expected Opening Tag!", EXPFILL }}, + { &ei_bacapp_max_recursion_depth_reached, { "bacapp.max_recursion_depth_reached", + PI_PROTOCOL, PI_WARN, "Maximum allowed recursion depth reached. Dissection stopped.", EXPFILL }} }; expert_module_t* expert_bacapp; -- 2.23.0