Wireshark-commits: [Wireshark-commits] master 9118d95: Qt/PacketList: read packet record in private

From: Wireshark code review <code-review-do-not-reply@xxxxxxxxxxxxx>
Date: Sun, 30 Sep 2018 16:30:13 +0000
URL: https://code.wireshark.org/review/gitweb?p=wireshark.git;a=commit;h=9118d959a4f6231a7e928161620e579467b610a9
Submitter: "Guy Harris <guy@xxxxxxxxxxxx>"
Changed: branch: master
Repository: wireshark

Commits:

9118d95 by Peter Wu (peter@xxxxxxxxxxxxx):

    Qt/PacketList: read packet record in private buffer
    
    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@xxxxxxxxxxxxx>
    Tested-by: Petri Dish Buildbot
    Reviewed-by: Guy Harris <guy@xxxxxxxxxxxx>
    

Actions performed:

    from  5a401cc   [Automatic update for 2018-09-30]
     add  9118d95   Qt/PacketList: read packet record in private buffer


Summary of changes:
 ui/qt/packet_list.cpp | 17 +++++++++++++----
 1 file changed, 13 insertions(+), 4 deletions(-)