Wireshark-commits: [Wireshark-commits] master 66af843: bacapp: make sure to NUL terminate bf_arr.

Date Prev · Date Next · Thread Prev · Thread Next
From: Wireshark code review <code-review-do-not-reply@xxxxxxxxxxxxx>
Date: Thu, 01 Feb 2018 01:01:38 +0000
URL: https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=66af843eb502a16a2428e899b44939cfe3925863
Submitter: Jakub Zawadzki (darkjames-ws@xxxxxxxxxxxx)
Changed: branch: master
Repository: wireshark

Commits:

66af843 by Jakub Zawadzki (darkjames@xxxxxxxxxxxx):

    bacapp: make sure to NUL terminate bf_arr.
    
    bf_arr is used as %s argument to proto_tree_add_subtree_format(), so it need to be NUL terminated.
    Add + 1 to bf_arr size, and use sizeof() in memset() calls.
    
    ASAN report:
    
    ERROR: AddressSanitizer: stack-buffer-overflow on address
    0x7ff1b179f150 at pc 0x00000044cf31 bp 0x7ffdc7493cf0 sp 0x7ffdc74934a0
    READ of size 258 at 0x7ff1b179f150 thread T0
    SCARINESS: 41 (multi-byte-read-stack-buffer-overflow)
    	#0 0x44cf30 in printf_common(void*, char const*, __va_list_tag*) /src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors_format.inc:548
    	#1 0x498cfc in __vsnprintf_chk /src/llvm/projects/compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc:1558
    	#2 0x5775cf in proto_tree_set_representation /src/wireshark/epan/proto.c:5508:9
    	#3 0x577eb1 in proto_tree_add_text_valist_internal /src/wireshark/epan/proto.c:1226:2
    	#4 0x5782d5 in proto_tree_add_subtree_format /src/wireshark/epan/proto.c:1249:7
    	#5 0x73c73f in fBitStringTagVS /src/wireshark/epan/dissectors/packet-bacapp.c:7490:15
    	#6 0x73ad20 in fApplicationTypesEnumeratedSplit /src/wireshark/epan/dissectors/packet-bacapp.c:7569:26
    	#7 0x73a484 in fApplicationTypes /src/wireshark/epan/dissectors/packet-bacapp.c:7635:12
    	#8 0x7395db in fIAmRequest /src/wireshark/epan/dissectors/packet-bacapp.c:13412:14
    	#9 0x7383e1 in dissect_bacapp /src/wireshark/epan/dissectors/packet-bacapp.c:14163:9
    
    Found by oss-fuzz/5452.
    
    Change-Id: I57e948904f707c5003a389431b009a37c1212e04
    Link: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=5452
    Reviewed-on: https://code.wireshark.org/review/25544
    Petri-Dish: Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx>
    Tested-by: Petri Dish Buildbot
    Reviewed-by: Jakub Zawadzki <darkjames-ws@xxxxxxxxxxxx>
    

Actions performed:

    from  85fed81   ieee80211: various fixes to the 802.11ax support.
    adds  66af843   bacapp: make sure to NUL terminate bf_arr.


Summary of changes:
 epan/dissectors/packet-bacapp.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)