From b5ea3d2c09c5df49ed783dabdc83820c55effdd2 Mon Sep 17 00:00:00 2001 From: Julian Eisel Date: Wed, 6 Oct 2021 11:20:15 +0200 Subject: [PATCH] Fix possible use-after-free when cancelling temporary rename button If a renaming button was removed via `UI_but_active_only_ex()` and that button was placed using the layout system, the button was still in the layout. So far this didn't cause issues, because all cases where the button may be removed were not using the layout system. --- source/blender/editors/interface/interface.c | 3 ++ .../editors/interface/interface_intern.h | 1 + .../editors/interface/interface_layout.c | 42 +++++++++++++++---- 3 files changed, 37 insertions(+), 9 deletions(-) diff --git a/source/blender/editors/interface/interface.c b/source/blender/editors/interface/interface.c index 9a294162f34..88b23e07f54 100644 --- a/source/blender/editors/interface/interface.c +++ b/source/blender/editors/interface/interface.c @@ -1014,6 +1014,9 @@ bool UI_but_active_only_ex( else if ((found == true) && (isactive == false)) { if (remove_on_failure) { BLI_remlink(&block->buttons, but); + if (but->layout) { + ui_layout_remove_but(but->layout, but); + } ui_but_free(C, but); } return false; diff --git a/source/blender/editors/interface/interface_intern.h b/source/blender/editors/interface/interface_intern.h index 8b45d9faae6..8e69ac40a34 100644 --- a/source/blender/editors/interface/interface_intern.h +++ b/source/blender/editors/interface/interface_intern.h @@ -1107,6 +1107,7 @@ void ui_resources_free(void); /* interface_layout.c */ void ui_layout_add_but(uiLayout *layout, uiBut *but); +void ui_layout_remove_but(uiLayout *layout, const uiBut *but); bool ui_layout_replace_but_ptr(uiLayout *layout, const void *old_but_ptr, uiBut *new_but); uiBut *ui_but_add_search(uiBut *but, PointerRNA *ptr, diff --git a/source/blender/editors/interface/interface_layout.c b/source/blender/editors/interface/interface_layout.c index 64c16e57f56..e54b261facd 100644 --- a/source/blender/editors/interface/interface_layout.c +++ b/source/blender/editors/interface/interface_layout.c @@ -5605,28 +5605,52 @@ void ui_layout_add_but(uiLayout *layout, uiBut *but) ui_button_group_add_but(uiLayoutGetBlock(layout), but); } -bool ui_layout_replace_but_ptr(uiLayout *layout, const void *old_but_ptr, uiBut *new_but) +static uiButtonItem *ui_layout_find_button_item(const uiLayout *layout, const uiBut *but) { - ListBase *child_list = layout->child_items_layout ? &layout->child_items_layout->items : - &layout->items; + const ListBase *child_list = layout->child_items_layout ? &layout->child_items_layout->items : + &layout->items; LISTBASE_FOREACH (uiItem *, item, child_list) { if (item->type == ITEM_BUTTON) { uiButtonItem *bitem = (uiButtonItem *)item; - if (bitem->but == old_but_ptr) { - bitem->but = new_but; - return true; + if (bitem->but == but) { + return bitem; } } else { - if (ui_layout_replace_but_ptr((uiLayout *)item, old_but_ptr, new_but)) { - return true; + uiButtonItem *nested_item = ui_layout_find_button_item((uiLayout *)item, but); + if (nested_item) { + return nested_item; } } } - return false; + return NULL; +} + +void ui_layout_remove_but(uiLayout *layout, const uiBut *but) +{ + uiButtonItem *bitem = ui_layout_find_button_item(layout, but); + if (!bitem) { + return; + } + + BLI_freelinkN(&layout->items, bitem); +} + +/** + * \return true if the button was successfully replaced. + */ +bool ui_layout_replace_but_ptr(uiLayout *layout, const void *old_but_ptr, uiBut *new_but) +{ + uiButtonItem *bitem = ui_layout_find_button_item(layout, old_but_ptr); + if (!bitem) { + return false; + } + + bitem->but = new_but; + return true; } void uiLayoutSetFixedSize(uiLayout *layout, bool fixed_size)