Skip to content

Commit

Permalink
Qt: Fix uninitialized memory access in val_units_
Browse files Browse the repository at this point in the history
Fixes the following UBSAN errors:

    ui/qt/io_graph_dialog.cpp:1720:75: runtime error: load of value 3200171710, which is not a valid value for type 'io_graph_item_unit_t'
        #0 0x5611f0b0cd1d in IOGraph::setFilter(QString const&) ui/qt/io_graph_dialog.cpp:1720:75
        wireshark#1 0x5611f0b737a1 in IOGraph::IOGraph(QCustomPlot*) ui/qt/io_graph_dialog.cpp:1682:5
        wireshark#2 0x5611f0afb3f3 in IOGraphDialog::addGraph(bool, QString, QString, int, IOGraph::PlotStyles, io_graph_item_unit_t, QString, int) ui/qt/io_graph_dialog.cpp:340:24
        wireshark#3 0x5611f0af7c19 in IOGraphDialog::IOGraphDialog(QWidget&, CaptureFile&) ui/qt/io_graph_dialog.cpp:289:13

    ui/qt/io_graph_dialog.cpp:1818:19: runtime error: load of value 3200171710, which is not a valid value for type 'io_graph_item_unit_t'
        #0 0x5611f0b1167e in IOGraph::setPlotStyle(int) ui/qt/io_graph_dialog.cpp:1818:19
        wireshark#1 0x5611f0b062ee in IOGraphDialog::syncGraphSettings(QTreeWidgetItem*) ui/qt/io_graph_dialog.cpp:420:10

    ui/qt/io_graph_dialog.cpp:1872:29: runtime error: load of value 3200171710, which is not a valid value for type 'io_graph_item_unit_t'
        #0 0x5611f0b13e6a in IOGraph::setValueUnits(int) ui/qt/io_graph_dialog.cpp:1872:29
        wireshark#1 0x5611f0b06640 in IOGraphDialog::syncGraphSettings(QTreeWidgetItem*) ui/qt/io_graph_dialog.cpp:422:10

Note that calling setFilter with an empty string is pretty useless,
especially since the filter is initialized later, so remove it.
The choice for IOG_ITEM_UNIT_FIRST is quite arbitrary and needed because
setValueUnits reads the "old" (uninitialized) value.

Change-Id: I32c65a30593cb718b838c0f324e0d1b0eaab90e5
Reviewed-on: https://code.wireshark.org/review/20767
Petri-Dish: Peter Wu <peter@lekensteyn.nl>
Tested-by: Petri Dish Buildbot <buildbot-no-reply@wireshark.org>
Reviewed-by: Peter Wu <peter@lekensteyn.nl>
  • Loading branch information
Lekensteyn committed Mar 30, 2017
1 parent aa6dcf9 commit 0ea51ad
Showing 1 changed file with 5 additions and 3 deletions.
8 changes: 5 additions & 3 deletions ui/qt/io_graph_dialog.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -415,11 +415,13 @@ void IOGraphDialog::syncGraphSettings(QTreeWidgetItem *item)

iog->setName(item->text(name_col_));

/* dfilter and plot style depend on the value unit, so set it first. */
iog->setValueUnits(item->data(yaxis_col_, Qt::UserRole).toInt());

iog->setFilter(item->text(dfilter_col_));
iog->setColor(colors_[item->data(color_col_, Qt::UserRole).toInt() % colors_.size()]);
iog->setPlotStyle(item->data(style_col_, Qt::UserRole).toInt());

iog->setValueUnits(item->data(yaxis_col_, Qt::UserRole).toInt());
iog->setValueUnitField(item->text(yfield_col_));

iog->moving_avg_period_ = item->data(sma_period_col_, Qt::UserRole).toUInt();
Expand Down Expand Up @@ -1650,6 +1652,7 @@ IOGraph::IOGraph(QCustomPlot *parent) :
visible_(false),
graph_(NULL),
bars_(NULL),
val_units_(IOG_ITEM_UNIT_FIRST),
hf_index_(-1),
cur_idx_(-1)
{
Expand All @@ -1668,10 +1671,9 @@ IOGraph::IOGraph(QCustomPlot *parent) :
if (error_string) {
// QMessageBox::critical(this, tr("%1 failed to register tap listener").arg(name_),
// error_string->str);
// config_err_ = error_string->str;
g_string_free(error_string, TRUE);
}

setFilter(QString());
}

IOGraph::~IOGraph() {
Expand Down

0 comments on commit 0ea51ad

Please sign in to comment.