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

[paliak]

I forgot to mention that the suggested patch is under MIT license.

>
> 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