[oss-fuzz] #13791 wireshark/fuzzshark_ip_proto-udp: Heap-buffer-overflow in g_array_append_vals
This issue was migrated from bug 15617 in our old bug tracker.
Original bug information:
Reporter: Dario Lombardo
Status: RESOLVED FIXED
Product: Wireshark
Component: Dissection engine (libwireshark)
OS: Linux
Platform: x86
Version: Git
Attachments:
clusterfuzz-testcase-fuzzshark_ip_proto-udp-5765508667801600.pcap: testcase
clusterfuzz-testcase-fuzzshark_ip_proto-udp-5765508667801600.pcap: Packet capture file
See also: http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-10896
Activity
Created attachment 16992
testcase
Build Information:
Paste the COMPLETE build information from "Help->About Wireshark", "wireshark -v", or "tshark -v".
A new bug has been found by oss-fuzz: ==1==ERROR: AddressSanitizer: heap-buffer-overflow on address 0x6030001d9c62 at pc 0x00000045b2d2 bp 0x7ffca81db4a0 sp 0x7ffca81dac50 READ of size 4 at 0x6030001d9c62 thread T0 SCARINESS: 17 (4-byte-read-heap-buffer-overflow) #0 0x45b2d1 in __interceptor_memcpy.part.38 /src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:801 #1 0x26d818e in g_array_append_vals #2 0x26d9198 in g_byte_array_append #3 0x6b999c in proto_tree_set_bytes /src/wireshark/epan/proto.c:3850:3 #4 0x6b999c in proto_tree_add_bytes /src/wireshark/epan/proto.c:3766 #5 0x6ba7d4 in proto_tree_add_bytes_format_value /src/wireshark/epan/proto.c:3808:7 #6 0xe3ac1d in oap_1_tree_add_alias /src/wireshark/epan/dissectors/packet-dof.c:1387:18 #7 0xe38827 in dissect_oap /src/wireshark/epan/dissectors/packet-dof.c:8440:22 #8 0x651a91 in call_dissector_through_handle /src/wireshark/epan/packet.c:706:9 #9 0x651a91 in call_dissector_work /src/wireshark/epan/packet.c:791 #10 0x651569 in dissector_try_uint_new /src/wireshark/epan/packet.c:1383:8 #11 0xe30c6a in dissect_app_common /src/wireshark/epan/dissectors/packet-dof.c:5314:13 #12 0xe30c6a in dissect_dpp_2 /src/wireshark/epan/dissectors/packet-dof.c:7277added crash libwireshark oslinux versiondev labels
-
Created attachment 17001 Packet capture file Reproduced with: TShark (Wireshark) 3.1.0 (v3.1.0rc0-361-ged40d318) -- A problem was found by the oss-fuzz project: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=13791 Attached is the sample that triggers this error which can be reproduced with an ASAN+UBSAN build of Wireshark: tshark -Vxr clusterfuzz-testcase-fuzzshark_ip_proto-udp-5765508667801600.pcap -- epan/dissectors/packet-dof.c:3624:5: runtime error: null pointer passed as argument 1, which is declared to never be null /usr/include/string.h:43:28: note: nonnull attribute specified here #0 0x7f9f534144a3 in assign_addr_port_id epan/dissectors/packet-dof.c:3624:5 #1 0x7f9f53419e05 in dissect_dof_udp epan/dissectors/packet-dof.c:5765:39 #2 0x7f9f56aebc55 in call_dissector_through_handle epan/packet.c:706:9 #3 0x7f9f56ad6958 in call_dissector_work epan/packet.c:791:9 #4 0x7f9f56ad59f8 in dissector_try_uint_new epan/packet.c:1383:8 #5 0x7f9f56ad71eb in dissector_try_uint epan/packet.c:1407:9 #6 0x7f9f55052b01 in decode_udp_ports epan/dissectors/packet-udp.c:685:7 #7 0x7f9f55065e99 in dissect epan/dissectors/packet-udp.c:1222:5 #8 0x7f9f5505772d in dissect_udp epan/dissectors/packet-udp.c:1228:3 #9 0x7f9f56aebc55 in call_dissector_through_handle epan/packet.c:706:9 #10 0x7f9f56ad6958 in call_dissector_work epan/packet.c:791:9 #11 0x7f9f56ad59f8 in dissector_try_uint_new epan/packet.c:1383:8 #12 0x7f9f535b80f5 in dissect_exported_pdu epan/dissectors/packet-exported_pdu.c:370:17 #13 0x7f9f56aebc55 in call_dissector_through_handle epan/packet.c:706:9 #14 0x7f9f56ad6958 in call_dissector_work epan/packet.c:791:9 #15 0x7f9f56ae4f3a in call_dissector_only epan/packet.c:3141:8 #16 0x7f9f536dc8e0 in dissect_frame epan/dissectors/packet-frame.c:623:6 #17 0x7f9f56aebc55 in call_dissector_through_handle epan/packet.c:706:9 #18 0x7f9f56ad6958 in call_dissector_work epan/packet.c:791:9 #19 0x7f9f56ae4f3a in call_dissector_only epan/packet.c:3141:8 #20 0x7f9f56ace764 in call_dissector_with_data epan/packet.c:3154:8 #21 0x7f9f56acdb1c in dissect_record epan/packet.c:580:3 #22 0x7f9f56a7ac08 in epan_dissect_run_with_taps epan/epan.c:563:2 #23 0x55c8a1a3337b in process_packet_single_pass tshark.c:3499:5 #24 0x55c8a1a2c585 in process_cap_file tshark.c:3332:11 #25 0x55c8a1a24555 in main tshark.c:2025:17 #26 0x7f9f47e3a222 in __libc_start_main (/usr/lib/libc.so.6+0x24222) #27 0x55c8a18c858d in _start (run/tshark+0xdd58d) -
That's odd, my trace is about a different issue than reported in the oss-fuzz trace (and the description of this bug). It does match the issue that is being fixed in https://code.wireshark.org/review/32442 though.
-
Change 32442 merged by Peter Wu: dof: don't use src/dst addresses if they don't exist.
-
Change 32558 had a related patch set uploaded by Peter Wu: dof: don't use src/dst addresses if they don't exist.
-
Change 32558 merged by Peter Wu: dof: don't use src/dst addresses if they don't exist.
-
Change 32559 had a related patch set uploaded by Peter Wu: dof: don't use src/dst addresses if they don't exist.
-
Change 32559 merged by Peter Wu: dof: don't use src/dst addresses if they don't exist.
-
Change 32560 had a related patch set uploaded by Peter Wu: dof: don't use src/dst addresses if they don't exist.
-
Change 32560 merged by Peter Wu: dof: don't use src/dst addresses if they don't exist.
-
Sigh, there appears to be two separate issues. The denial of service (null-pointer dereference crash) fix is fixed in: v3.1.0rc0-374-g1ce2918f v3.0.1rc0-59-gf5736b0b v2.6.8rc0-26-g0ba00612 v2.4.14rc0-23-ge5aed6ea
There is possibly another buffer-overflow (read) which is not yet fixed.
-
The buffer overflow (read) occurs in the first proto_tree_add_bytes_format_value call: /* Decode the Interface */ ti = proto_tree_add_bytes_format_value(tree, hf_oap_1_interfaceid, tvb, offset, alias_length, binding->iid, "%s", dof_iid_create_standard_string(binding->iid_length, binding->iid)); PROTO_ITEM_SET_GENERATED(ti); /* Decode the Object ID */ ti = proto_tree_add_bytes_format_value(tree, hf_oap_1_objectid, tvb, offset, alias_length, binding->oid, "%s", dof_oid_create_standard_string(binding->oid_length, binding->oid)); If alias_length is larger than binding->iid_length, then a buffer overflow (read) will occur. Changing alias_length to binding->iid_length (or oid_length in the case below) could potentially work, but the selected bytes could be wrong. Do you want to investigate this and look for the correct behavior (e.g. based on a spec)? -
(In reply to Peter Wu from comment #11) > The buffer overflow (read) occurs in the first > proto_tree_add_bytes_format_value call: > > /* Decode the Interface */ > ti = proto_tree_add_bytes_format_value(tree, > hf_oap_1_interfaceid, tvb, offset, alias_length, binding->iid, "%s", > dof_iid_create_standard_string(binding->iid_length, binding->iid)); > PROTO_ITEM_SET_GENERATED(ti); > > /* Decode the Object ID */ > ti = proto_tree_add_bytes_format_value(tree, hf_oap_1_objectid, > tvb, offset, alias_length, binding->oid, "%s", > dof_oid_create_standard_string(binding->oid_length, binding->oid)); > > If alias_length is larger than binding->iid_length, then a buffer overflow > (read) will occur. Changing alias_length to binding->iid_length (or > oid_length in the case below) could potentially work, but the selected bytes > could be wrong. Do you want to investigate this and look for the correct > behavior (e.g. based on a spec)? According to the OAP v1 spec[1], the alias and binding data are separate. It looks like we should be doing something like proto_tree_add_bytes_format_value(tree, hf_oap_1_interfaceid, tvb, offset + alias_length, binding->iid_length, ... [1]https://asset.opendof.org/content/org.opendof.core-specifications/specifications/7/Object_Access_Protocol_V1.pdf -
Change 32698 had a related patch set uploaded by Gerald Combs: DOF: Fix alias binding dissection.
-
Change 32723 had a related patch set uploaded by Gerald Combs: DOF: Fix alias binding dissection.
-
Change 32698 merged by Gerald Combs: DOF: Fix alias binding dissection.
-
Change 32723 merged by Gerald Combs: DOF: Fix alias binding dissection.
-
Change 32724 had a related patch set uploaded by Gerald Combs: DOF: Fix alias binding dissection.
-
Change 32725 had a related patch set uploaded by Gerald Combs: DOF: Fix alias binding dissection.