AuroraMiddleware/qt5base-lts
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

QFormLayout: fix use-after-free in clearQLayoutItem()

Browse Source 
Found by ASan when it should have been found by me in the initial
review...

The old code did, in this order:

1. delete item->layout() (which deletes item, as QLayoutItem::layout()
   is just a dynamic_cast implemented with virtual functions)

2. delete item->widget() (which is correct, but too late, as 'item'
   may have already been deleted; this is the first use-after-free
   bug)

3. delete item->spacerItem() (which is the second use-after-free).

Fix by deleting item->widget() (which may be a no-op), _then_ checking
for a QLayout, deleting all children (but not the layout), and finally
deleting item as a QLayoutItem.

Rename clearQLayoutItem() to clearAndDestroyQLayoutItem() to better
match what it actually does.

Change-Id: Id70a7a137dac5de466ef619f01bfd61dfc150418
Reviewed-by: Thiago Macieira <thiago.macieira@intel.com>
Reviewed-by: Samuel Gaist <samuel.gaist@edeltech.ch>
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
This commit is contained in:
Marc Mutz 2016-09-27 15:30:15 +02:00
parent 7920cfe46a
commit 34c2a1dcf0
1 changed files with 12 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

27
src/widgets/kernel/qformlayout.cpp
View File

@ -1410,18 +1410,15 @@ static QLayoutItem *ownershipCleanedItem(QFormLayoutItem *item, QFormLayout *lay
return i;
}
static void clearQLayoutItem(QLayoutItem *item)
static void clearAndDestroyQLayoutItem(QLayoutItem *item)
{
if (Q_LIKELY(item)) {
if (QLayout *layout = item->layout()) {
while (QLayoutItem *child = layout->takeAt(0)) {
clearQLayoutItem(child);
delete child;
}
delete layout;
}
delete item->widget();
delete item->spacerItem();
if (QLayout *layout = item->layout()) {
while (QLayoutItem *child = layout->takeAt(0))
clearAndDestroyQLayoutItem(child);
}
delete item;
}
}
@ -1453,8 +1450,8 @@ static void clearQLayoutItem(QLayoutItem *item)
void QFormLayout::removeRow(int row)
{
TakeRowResult result = takeRow(row);
clearQLayoutItem(result.labelItem);
clearQLayoutItem(result.fieldItem);
clearAndDestroyQLayoutItem(result.labelItem);
clearAndDestroyQLayoutItem(result.fieldItem);
}
/*!
@ -1485,8 +1482,8 @@ void QFormLayout::removeRow(int row)
void QFormLayout::removeRow(QWidget *widget)
{
TakeRowResult result = takeRow(widget);
clearQLayoutItem(result.labelItem);
clearQLayoutItem(result.fieldItem);
clearAndDestroyQLayoutItem(result.labelItem);
clearAndDestroyQLayoutItem(result.fieldItem);
}
/*!
@ -1518,8 +1515,8 @@ void QFormLayout::removeRow(QWidget *widget)
void QFormLayout::removeRow(QLayout *layout)
{
TakeRowResult result = takeRow(layout);
clearQLayoutItem(result.labelItem);
clearQLayoutItem(result.fieldItem);
clearAndDestroyQLayoutItem(result.labelItem);
clearAndDestroyQLayoutItem(result.fieldItem);
}
/*!