osmocom
/
wireshark
11
0
Fork 1
Code Issues Pull Requests Projects Releases Wiki Activity

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@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Guy Harris <guy@alum.mit.edu>
This commit is contained in:
Peter Wu 2018-09-28 13:03:22 +02:00 committed by Guy Harris
parent 5a401ccad9
commit 9118d959a4
1 changed files with 12 additions and 3 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
ui/qt/packet_list.cpp
View File

@ -1037,15 +1037,22 @@ QString PacketList::getFilterFromRowAndColumn()
if (fdata != NULL) {
epan_dissect_t edt;
wtap_rec rec; /* Record metadata */
Buffer buf; /* Record data */
if (!cf_read_record(cap_file_, fdata))
wtap_rec_init(&rec);
ws_buffer_init(&buf, 1500);
if (!cf_read_record_r(cap_file_, fdata, &rec, &buf)) {
wtap_rec_cleanup(&rec);
ws_buffer_free(&buf);
return filter; /* error reading the record */
}
/* proto tree, visible. We need a proto tree if there's custom columns */
epan_dissect_init(&edt, cap_file_->epan, have_custom_cols(&cap_file_->cinfo), FALSE);
col_custom_prime_edt(&edt, &cap_file_->cinfo);
epan_dissect_run(&edt, cap_file_->cd_t, &cap_file_->rec,
frame_tvbuff_new_buffer(&cap_file_->provider, fdata, &cap_file_->buf),
epan_dissect_run(&edt, cap_file_->cd_t, &rec,
frame_tvbuff_new_buffer(&cap_file_->provider, fdata, &buf),
fdata, &cap_file_->cinfo);
epan_dissect_fill_in_columns(&edt, TRUE, TRUE);
@ -1094,6 +1101,8 @@ QString PacketList::getFilterFromRowAndColumn()
}
epan_dissect_cleanup(&edt);
wtap_rec_cleanup(&rec);
ws_buffer_free(&buf);
}
return filter;