[vbox-dev] Crash when right-clicking on a group with both a machine and a group selected.

[paliak]

Hi,

When using the Qt front end; selecting both a machine and a group,
then right-clicking on a group, causes a segfault.

This seems to be caused by a switch statement fall through in:
    vbox/src/VBox/Frontends/VirtualBox/src/manager/chooser/UIChooserModel.cpp:1661,1703

itemAt(pEvent->scenePos()) correctly returns the group item (Type:
UIChooserNodeType_Group) and the switch statement goes into the
correct block (case UIChooserNodeType_Group) but because the following
if statement fails (more than one item selected, one of the selected
items is a group)

    if (selectedItems().contains(pGroupItem) && selectedItems().size() == 1)

it falls through into the UIChooserNodeType_Machine case.

    qgraphicsitem_cast<UIChooserItemMachine*>(pItem);

returns null in agreement with the qt docs since pItem is not of type
UIChooserItemMachine causing pMachineItem to be null which in turn
causes

    pMachineItem->cacheType()

to trigger a SIGSEGV. Backtrace:

(gdb) backtrace
#0  0x0000555555671804 in UIChooserItem::node() const (this=0x0) at
/home/dev/Documents/vbox/src/VBox/Frontends/VirtualBox/src/manager/chooser/UIChooserItem.h:118
#1  0x000055555568dab6 in UIChooserItemMachine::nodeToMachineType()
const (this=0x0)
    at /home/dev/Documents/vbox/src/VBox/Frontends/VirtualBox/src/manager/chooser/UIChooserItemMachine.cpp:71
#2  0x000055555568db88 in UIChooserItemMachine::cache() const
(this=0x0) at /home/dev/Documents/vbox/src/VBox/Frontends/VirtualBox/src/manager/chooser/UIChooserItemMachine.cpp:86
#3  0x000055555568dbc2 in UIChooserItemMachine::cacheType() const
(this=0x0) at /home/dev/Documents/vbox/src/VBox/Frontends/VirtualBox/src/manager/chooser/UIChooserItemMachine.cpp:91
#4  0x000055555566fbe6 in
UIChooserModel::processContextMenuEvent(QGraphicsSceneContextMenuEvent*)
(this=0x555557012fb0, pEvent=0x7fffffffce20)
    at /home/dev/Documents/vbox/src/VBox/Frontends/VirtualBox/src/manager/chooser/UIChooserModel.cpp:1694

The suggested patch is to check if pMachineItem is null before any
calls to pMachineItem->cacheType() and break if so.

Index: src/VBox/Frontends/VirtualBox/src/manager/chooser/UIChooserModel.cpp
===================================================================
--- src/VBox/Frontends/VirtualBox/src/manager/chooser/UIChooserModel.cpp
(revision 100336)
+++ src/VBox/Frontends/VirtualBox/src/manager/chooser/UIChooserModel.cpp
(working copy)
@@ -1690,8 +1690,9 @@
                     {
                         /* Get machine-item: */
                         UIChooserItemMachine *pMachineItem =
qgraphicsitem_cast<UIChooserItemMachine*>(pItem);
+                        if (!pMachineItem) break;
                         /* Machine context menu for other
Group/Machine cases: */
-                        if (pMachineItem->cacheType() ==
UIVirtualMachineItemType_Local)
+                        else if (pMachineItem->cacheType() ==
UIVirtualMachineItemType_Local)

m_localMenus.value(UIChooserNodeType_Machine)->exec(pEvent->screenPos());
                         else if (pMachineItem->cacheType() ==
UIVirtualMachineItemType_CloudReal)

m_cloudMenus.value(UIChooserNodeType_Machine)->exec(pEvent->screenPos());

This could also be fixed by checking type of pItem inside the
mentioned if statement.

I hope I haven't missed anything in my patch submission; first time
using a mailing list.

Regards.

Paliak