diff options
| author | Darius Davis <darius-wireshark@free-range.com.au> | 2024-02-21 11:34:59 +1000 |
|---|---|---|
| committer | Darius Davis <darius-wireshark@free-range.com.au> | 2024-02-21 12:45:19 +1000 |
| commit | c99e37131b6aeec65380b420d79944c6de4352ba (patch) | |
| tree | 696bd44f213aa4375c8655c8bb1d59fc9932e0f9 | |
| parent | b8c6c9e1a8fae2b2de616110968ae55ec48a0ca3 (diff) | |
TPNCP: Fix two potential array overruns.
The TPNCP dissector depends upon a resource file, tpncp.dat, being loaded
during initialization. If a non-default tpncp.dat was used, the TPNCP
dissector could potentially perform some operations beyond the bounds of a
fixed-size array while loading tpncp.dat.
If a non-default tpncp.dat was used and an attempt was made to dissect
malformed TPNCP traffic, the TPNCP dissector could potentially perform a read
beyond the end of an array.
This change adds explicit bounds-checks to eliminate these possible OOB
accesses.
There is zero chance of this being triggered in a default unmodified
installation of Wireshark: Loading of the tpncp.dat file is conditional on a
preference setting which defaults to FALSE, and even if it is configured to
TRUE, the included tpncp.dat does not trigger either of these OOB operations.
It still seems worthwhile to make the parser and dissector generally more
robust.
| -rw-r--r-- | epan/dissectors/packet-tpncp.c | 8 |
1 files changed, 6 insertions, 2 deletions
diff --git a/epan/dissectors/packet-tpncp.c b/epan/dissectors/packet-tpncp.c index ed7c8fa97a..e5e7c1a8a6 100644 --- a/epan/dissectors/packet-tpncp.c +++ b/epan/dissectors/packet-tpncp.c @@ -313,7 +313,7 @@ dissect_tpncp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U if (len > 8) proto_tree_add_int(tpncp_tree, hf_tpncp_cid, tvb, 12, 4, cid); offset += 16; - if (tpncp_events_info_db[id].size && len > 12) { + if (id < MAX_TPNCP_DB_SIZE && tpncp_events_info_db[id].size && len > 12) { event_tree = proto_tree_add_subtree_format( tree, tvb, offset, -1, ett_tpncp_body, NULL, "TPNCP Event: %s (%d)", @@ -330,7 +330,7 @@ dissect_tpncp(tvbuff_t *tvb, packet_info *pinfo, proto_tree *tree, void *data _U if (try_val_to_str(id, tpncp_commands_id_vals)) { proto_tree_add_uint(tpncp_tree, hf_tpncp_command_id, tvb, 8, 4, id); offset += 12; - if (tpncp_commands_info_db[id].size && len > 8) { + if (id < MAX_TPNCP_DB_SIZE && tpncp_commands_info_db[id].size && len > 8) { command_tree = proto_tree_add_subtree_format( tree, tvb, offset, -1, ett_tpncp_body, NULL, "TPNCP Command: %s (%d)", @@ -812,6 +812,10 @@ init_tpncp_data_fields_info(tpncp_data_field_info *data_fields_info, FILE *file) is_address_family = FALSE; if (current_data_id != data_id) { /* new data */ + if (data_id >= MAX_TPNCP_DB_SIZE) { + report_failure("ERROR! The data_id %d is too large.", data_id); + continue; + } if (registered_struct_ids[data_id] == TRUE) { report_failure( "ERROR! The data_id %d already registered. Cannot register two identical events/command", |