Qt: fix use-after-free on error while saving exported packets

When an error occurs while saving packets using the Export Specified
Packets dialog (e.g. try to overwrite the opened capture file), the
dialog is displayed again. As PacketRangeGroupBox freed the packet
selection range, a crash (use-after-free) occurs.

Removes some unnecessary code in MainWindow::exportDissections as well.

Change-Id: I63898427eff7e71799d89c8a22246db8f93a9ff6
Fixes: v2.5.0rc0-968-g38b40acb2d ("Qt: fix a memory leak when exporting packets")
Reviewed-on: https://code.wireshark.org/review/27695
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot
Reviewed-by: Stig Bjørlykke <stig@bjorlykke.org>
Reviewed-by: Anders Broman <a.broman58@gmail.com>
This commit is contained in:
Peter Wu 2018-05-21 22:33:37 +02:00 committed by Anders Broman
parent 0bfdb0f72b
commit b078310bd0
9 changed files with 27 additions and 18 deletions

1
file.c
View File

@ -2114,6 +2114,7 @@ cf_retap_packets(capture_file *cf)
"all packets", TRUE, retap_packet,
&callback_args, TRUE);
packet_range_cleanup(&range);
epan_dissect_cleanup(&callback_args.edt);
cf_callback_invoke(cf_cb_file_retap_finished, cf);

View File

@ -225,6 +225,10 @@ void packet_range_init(packet_range_t *range, capture_file *cf) {
packet_range_calc_user(range);
}
void packet_range_cleanup(packet_range_t *range) {
wmem_free(NULL, range->user_range);
}
/* check whether the packet range is OK */
convert_ret_t packet_range_check(packet_range_t *range) {
if (range->process == range_process_user_range && range->user_range == NULL) {

View File

@ -84,6 +84,9 @@ typedef enum {
/* init the range structure */
extern void packet_range_init(packet_range_t *range, capture_file *cf);
/* Cleanup the range structure before the caller frees "range". */
extern void packet_range_cleanup(packet_range_t *range);
/* check whether the packet range is OK */
extern convert_ret_t packet_range_check(packet_range_t *range);

View File

@ -137,6 +137,7 @@ ExportDissectionDialog::~ExportDissectionDialog()
{
#if !defined(Q_OS_WIN)
g_free(print_args_.file);
packet_range_cleanup(&print_args_.range);
#endif
}

View File

@ -1575,7 +1575,7 @@ void MainWindow::exportSelectedPackets() {
case CANCELLED:
/* The user said "forget it". Just get rid of the dialog box
and return. */
return;
goto cleanup;
}
/*
@ -1632,7 +1632,7 @@ void MainWindow::exportSelectedPackets() {
packet_list_queue_draw();
/* Add this filename to the list of recent files in the "Recent Files" submenu */
add_menu_recent_capture_file(qUtf8Printable(file_name));
return;
goto cleanup;
case CF_WRITE_ERROR:
/* The save failed; let the user try again. */
@ -1640,24 +1640,19 @@ void MainWindow::exportSelectedPackets() {
case CF_WRITE_ABORTED:
/* The user aborted the save; just return. */
return;
goto cleanup;
}
}
return;
cleanup:
packet_range_cleanup(&range);
}
void MainWindow::exportDissections(export_type_e export_type) {
ExportDissectionDialog ed_dlg(this, capture_file_.capFile(), export_type);
packet_range_t range;
if (!capture_file_.capFile())
return;
/* Init the packet range */
packet_range_init(&range, capture_file_.capFile());
range.process_filtered = TRUE;
range.include_dependents = TRUE;
capture_file *cf = capture_file_.capFile();
g_return_if_fail(cf);
ExportDissectionDialog ed_dlg(this, cf, export_type);
ed_dlg.exec();
}

View File

@ -2109,8 +2109,10 @@ void MainWindow::on_actionStatisticsHpfeeds_triggered()
void MainWindow::on_actionFilePrint_triggered()
{
PrintDialog pdlg(this, capture_file_.capFile());
capture_file *cf = capture_file_.capFile();
g_return_if_fail(cf);
PrintDialog pdlg(this, cf);
pdlg.exec();
}

View File

@ -25,8 +25,6 @@ PacketRangeGroupBox::PacketRangeGroupBox(QWidget *parent) :
PacketRangeGroupBox::~PacketRangeGroupBox()
{
if (range_)
wmem_free(NULL, range_->user_range);
delete pr_ui_;
}

View File

@ -23,6 +23,10 @@ namespace Ui {
class PacketRangeGroupBox;
}
/**
* UI element for controlling a range selection. The range provided in
* "initRange" is not owned by this class but will be modified.
*/
class PacketRangeGroupBox : public QGroupBox
{
Q_OBJECT

View File

@ -68,7 +68,7 @@ PrintDialog::PrintDialog(QWidget *parent, capture_file *cf) :
page_pos_(0),
in_preview_(FALSE)
{
if (!cf) done(QDialog::Rejected); // ...or assert?
Q_ASSERT(cf);
pd_ui_->setupUi(this);
setWindowTitle(wsApp->windowTitleString(tr("Print")));
@ -123,6 +123,7 @@ PrintDialog::PrintDialog(QWidget *parent, capture_file *cf) :
PrintDialog::~PrintDialog()
{
packet_range_cleanup(&print_args_.range);
delete pd_ui_;
}