Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Merge pull request #2 from wireshark/master #9

Closed
wants to merge 1 commit into from
Closed

Merge pull request #2 from wireshark/master #9

wants to merge 1 commit into from

Conversation

bwhitn
Copy link
Contributor

@bwhitn bwhitn commented Feb 17, 2017

update

@bwhitn bwhitn closed this Feb 17, 2017
@bwhitn bwhitn reopened this Feb 17, 2017
@bwhitn bwhitn closed this Feb 17, 2017
@bwhitn bwhitn reopened this Feb 17, 2017
@bwhitn bwhitn closed this Feb 17, 2017
ghost pushed a commit that referenced this pull request Feb 1, 2018
@jwzawadzki
bacapp: make sure to NUL terminate bf_arr.
66af843 
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@darkjames.pl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Jakub Zawadzki <darkjames-ws@darkjames.pl>
ghost pushed a commit that referenced this pull request Sep 30, 2018
@Lekensteyn @guyharris
Qt/PacketList: read packet record in private buffer
9118d95 
To prevent potential interference with other users of the capture file,
read data in a private buffer instead of reusing the one from capFile.

An accidental (?) change in commit v2.9.0rc0-2001-g123bcb0362 resulted
in "cf_read_record" reallocating the capture_file->buf buffer. That
issue combined with the current behavior would result in a crash when
ignoring a packet followed by two times opening a context menu:

    ==32187==ERROR: AddressSanitizer: heap-use-after-free on address 0x7fda91642800 at pc 0x55a98f3faaa7 bp 0x7fffa2807860 sp 0x7fffa2807858
    READ of size 1 at 0x7fda91642800 thread T0
        #0 0x55a98f3faaa6 in QByteArray::operator[](int) const /usr/include/qt/QtCore/qbytearray.h:476:47
        #1 0x55a9901006eb in ByteViewText::drawLine(QPainter*, int, int) ui/qt/widgets/byte_view_text.cpp:370:35
        #2 0x55a9900fd109 in ByteViewText::paintEvent(QPaintEvent*) ui/qt/widgets/byte_view_text.cpp:217:9
        ...
        #50 0x55a98e9fd32a in PacketList::contextMenuEvent(QContextMenuEvent*) ui/qt/packet_list.cpp:614:15
        ...

    0x7fda91642800 is located 0 bytes inside of 3038371-byte region [0x7fda91642800,0x7fda919284a3)
    freed by thread T0 here:
        #0 0x55a98e65fd99 in __interceptor_realloc (run/wireshark+0x1019d99)
        #1 0x7fdac6e1bb88 in g_realloc /build/src/glib/glib/gmem.c:164
        #2 0x7fdaac12c908 in wtap_read_packet_bytes wiretap/wtap.c:1368:2
        #3 0x7fdaabf01e5a in libpcap_read_packet wiretap/libpcap.c:789:7
        #4 0x7fdaabef887d in libpcap_seek_read wiretap/libpcap.c:690:7
        #5 0x7fdaac12d5f5 in wtap_seek_read wiretap/wtap.c:1431:7
        #6 0x55a98e6c8611 in cf_read_record_r file.c:1566:8
        #7 0x55a98e6c88c5 in cf_read_record file.c:1576:10
        #8 0x55a98ea0b725 in PacketList::getFilterFromRowAndColumn() ui/qt/packet_list.cpp:1041:14
        #9 0x55a98e94e4a1 in MainWindow::setMenusForSelectedPacket() ui/qt/main_window_slots.cpp:1175:39

    previously allocated by thread T0 here:
        #0 0x55a98e65fd99 in __interceptor_realloc (run/wireshark+0x1019d99)
        #1 0x7fdac6e1bb88 in g_realloc /build/src/glib/glib/gmem.c:164
        #2 0x7fdaac12c908 in wtap_read_packet_bytes wiretap/wtap.c:1368:2
        #3 0x7fdaabf01e5a in libpcap_read_packet wiretap/libpcap.c:789:7
        #4 0x7fdaabef887d in libpcap_seek_read wiretap/libpcap.c:690:7
        #5 0x7fdaac12d5f5 in wtap_seek_read wiretap/wtap.c:1431:7
        #6 0x55a98e6c8611 in cf_read_record_r file.c:1566:8
        #7 0x55a98e6c88c5 in cf_read_record file.c:1576:10
        #8 0x55a98e6e0bde in cf_select_packet file.c:3777:8
        #9 0x55a98e9ea2ff in PacketList::selectionChanged(QItemSelection const&, QItemSelection const&) ui/qt/packet_list.cpp:420:9

This should be fixed now by I4f1264a406a28c79491dcd77c552193bf3cdf62d,
but let's avoid the shared buffer. It's not exactly a hot code path
anyway.

Change-Id: I548d7293a822601f4eb882672477540f066a066b
Reviewed-on: https://code.wireshark.org/review/29921
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Guy Harris <guy@alum.mit.edu>
ghost pushed a commit that referenced this pull request Nov 6, 2019
@velichkov @rknall
extcap: Fix memory leak in extcap_get_extcap_paths
1675e0f 
The path returned by get_persconffile_path needs to be freed.

Direct leak of 64 byte(s) in 1 object(s) allocated from:
    #0 0x5653e6c98e06 in realloc (/home/vasko/sources/wireshark/build_clang/run/wireshark+0x2486e06)
    #1 0x7f5b697f2e7d in g_realloc gmem.c:164:16
    #2 0x7f5b69810016 in g_string_maybe_expand gstring.c:102:21
    #3 0x7f5b69810369  gstring.c:476:7
    #4 0x7f5b69810369 in g_string_insert_len gstring.c:424:1
    #5 0x7f5b697d808d in g_build_path_va gfileutils.c:1766:7
    #6 0x7f5b697d9518 in g_build_filename_va gfileutils.c:1987:9
    #7 0x7f5b697d9518 in g_build_filename gfileutils.c:2069:9
    #8 0x7f5b69bd0c28 in get_persconffile_path /home/vasko/sources/wireshark/wsutil/filesystem.c:1856:12
    #9 0x5653e8825f82 in extcap_get_extcap_paths /home/vasko/sources/wireshark/extcap.c:258:53
    #10 0x5653e8825f82 in extcap_run_all /home/vasko/sources/wireshark/extcap.c:449
    #11 0x5653e8825f82 in extcap_load_interface_list /home/vasko/sources/wireshark/extcap.c:2024
    #12 0x5653e7775356 in main /home/vasko/sources/wireshark/ui/qt/main.cpp:726:5

Change-Id: I275d0ad6f06fbf3222c2d4ebef7f3079073404a0
Reviewed-on: https://code.wireshark.org/review/34994
Petri-Dish: Roland Knall <rknall@gmail.com>
Tested-by: Petri Dish Buildbot
Reviewed-by: Roland Knall <rknall@gmail.com>
@repo-lockdown
Copy link

repo-lockdown bot commented Apr 7, 2020

Hi, thank you for your contribution!

GitHub is however not the right place for these, please have a look at
https://www.wireshark.org/docs/wsdg_html_chunked/ChSrcContribute.html
for further instructions.

@repo-lockdown repo-lockdown bot locked and limited conversation to collaborators Apr 7, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
1 participant
@bwhitn