From ab7375dc6ba813df099adda4856f56c337f4a00b Mon Sep 17 00:00:00 2001 From: Guy Harris Date: Sat, 27 Feb 2021 14:25:55 -0800 Subject: [PATCH] Check for CaptureFileDialog::selectedFileType() failing. Have it return WTAP_FILE_TYPE_SUBTYPE_UNKNOWN, rather than an undecorated -1, if the hash table lookup fails. Check for that as a return value, and pop up a "file an issue" dialog if WTAP_FILE_TYPE_SUBTYPE_UNKNOWN is returned. This should squelch Coverity CID 1473325; the error Coverity reports is bogus, as negative file type/subtype values are check for before we try to use them as suffixes, but this should catch the "this should not happen" case that caused the error to pop up. --- ui/qt/capture_file_dialog.cpp | 34 +++++++++++++++++++++++++++++++--- ui/qt/main_window.cpp | 20 ++++++++++++++++++++ 2 files changed, 51 insertions(+), 3 deletions(-) diff --git a/ui/qt/capture_file_dialog.cpp b/ui/qt/capture_file_dialog.cpp index a5834cb2ca..51128a696d 100644 --- a/ui/qt/capture_file_dialog.cpp +++ b/ui/qt/capture_file_dialog.cpp @@ -625,7 +625,7 @@ void CaptureFileDialog::addMergeControls(QVBoxLayout &v_box) { } int CaptureFileDialog::selectedFileType() { - return type_hash_.value(selectedNameFilter(), -1); + return type_hash_.value(selectedNameFilter(), WTAP_FILE_TYPE_SUBTYPE_UNKNOWN); } wtap_compression_type CaptureFileDialog::compressionType() { @@ -748,8 +748,22 @@ check_savability_t CaptureFileDialog::saveAs(QString &file_name, bool must_suppo connect(this, &QFileDialog::filterSelected, this, &CaptureFileDialog::fixFilenameExtension); if (WiresharkFileDialog::exec() && selectedFiles().length() > 0) { + int file_type; + file_name = selectedFiles()[0]; - return checkSaveAsWithComments(this, cap_file_, selectedFileType()); + file_type = selectedFileType(); + /* Is the file type bogus? */ + if (file_type == WTAP_FILE_TYPE_SUBTYPE_UNKNOWN) { + /* This "should not happen". */ + QMessageBox msg_dialog; + + msg_dialog.setIcon(QMessageBox::Critical); + msg_dialog.setText(tr("Unknown file type returned by save as dialog.")); + msg_dialog.setInformativeText(tr("Please report this as a Wireshark issue at https://gitlab.com/wireshark/wireshark/-/issues.")); + msg_dialog.exec(); + return CANCELLED; + } + return checkSaveAsWithComments(this, cap_file_, file_type); } return CANCELLED; } @@ -784,8 +798,22 @@ check_savability_t CaptureFileDialog::exportSelectedPackets(QString &file_name, connect(this, &QFileDialog::filterSelected, this, &CaptureFileDialog::fixFilenameExtension); if (WiresharkFileDialog::exec() && selectedFiles().length() > 0) { + int file_type; + file_name = selectedFiles()[0]; - return checkSaveAsWithComments(this, cap_file_, selectedFileType()); + file_type = selectedFileType(); + /* Is the file type bogus? */ + if (file_type == WTAP_FILE_TYPE_SUBTYPE_UNKNOWN) { + /* This "should not happen". */ + QMessageBox msg_dialog; + + msg_dialog.setIcon(QMessageBox::Critical); + msg_dialog.setText(tr("Unknown file type returned by save as dialog.")); + msg_dialog.setInformativeText(tr("Please report this as a Wireshark issue at https://gitlab.com/wireshark/wireshark/-/issues.")); + msg_dialog.exec(); + return CANCELLED; + } + return checkSaveAsWithComments(this, cap_file_, file_type); } return CANCELLED; } diff --git a/ui/qt/main_window.cpp b/ui/qt/main_window.cpp index 9a13c1f901..24906c7ebd 100644 --- a/ui/qt/main_window.cpp +++ b/ui/qt/main_window.cpp @@ -1470,6 +1470,16 @@ bool MainWindow::saveAsCaptureFile(capture_file *cf, bool must_support_comments, return false; } file_type = save_as_dlg.selectedFileType(); + if (file_type == WTAP_FILE_TYPE_SUBTYPE_UNKNOWN) { + /* This "should not happen". */ + QMessageBox msg_dialog; + + msg_dialog.setIcon(QMessageBox::Critical); + msg_dialog.setText(tr("Unknown file type returned by merge dialog.")); + msg_dialog.setInformativeText(tr("Please report this as a Wireshark issue at https://gitlab.com/wireshark/wireshark/-/issues.")); + msg_dialog.exec(); + return false; + } compression_type = save_as_dlg.compressionType(); #ifdef Q_OS_WIN @@ -1604,6 +1614,16 @@ void MainWindow::exportSelectedPackets() { } file_type = esp_dlg.selectedFileType(); + if (file_type == WTAP_FILE_TYPE_SUBTYPE_UNKNOWN) { + /* This "should not happen". */ + QMessageBox msg_box; + + msg_box.setIcon(QMessageBox::Critical); + msg_box.setText(tr("Unknown file type returned by export dialog.")); + msg_box.setInformativeText(tr("Please report this as a Wireshark issue at https://gitlab.com/wireshark/wireshark/-/issues.")); + msg_box.exec(); + goto cleanup; + } compression_type = esp_dlg.compressionType(); #ifdef Q_OS_WIN // the Windows dialog does not fixup extensions, do it manually here.