From 7058a5dd8f086ae762abb2cb9f689951125f96cd Mon Sep 17 00:00:00 2001 From: LarsGit223 Date: Mon, 18 Feb 2019 20:41:11 +0100 Subject: [PATCH] scope: prevent usage of invalid iters (GtkTreeeIter) This is done by checking all results from calls to 'gtk_tree_selection_get_selected()'. Additionally some initializations have been moved behind parameter checks in 'scptreestore.c' to prevent crashes in case (still) an invalid iter would be passed as an argument. The changes in function 'stack_entry()' fixed #824. --- scope/src/break.c | 20 +++-- scope/src/inspect.c | 156 +++++++++++++++++++-------------- scope/src/local.c | 8 +- scope/src/memory.c | 16 ++-- scope/src/menu.c | 70 ++++++++------- scope/src/register.c | 20 +++-- scope/src/stack.c | 32 ++++--- scope/src/store/scptreestore.c | 15 ++-- scope/src/thread.c | 16 ++-- 9 files changed, 209 insertions(+), 144 deletions(-) diff --git a/scope/src/break.c b/scope/src/break.c index 0bb1cde8a..746d8ebcd 100644 --- a/scope/src/break.c +++ b/scope/src/break.c @@ -1332,8 +1332,10 @@ static void on_break_apply(const MenuItem *menu_item) if (menu_item || thread_id) { GtkTreeIter iter; - gtk_tree_selection_get_selected(selection, NULL, &iter); - break_apply(&iter, !menu_item); + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) + { + break_apply(&iter, !menu_item); + } } else plugin_beep(); @@ -1343,16 +1345,20 @@ static void on_break_run_apply(const MenuItem *menu_item) { GtkTreeIter iter; - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_set(store, &iter, BREAK_RUN_APPLY, - gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menu_item->widget)), -1); + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) + { + scp_tree_store_set(store, &iter, BREAK_RUN_APPLY, + gtk_check_menu_item_get_active(GTK_CHECK_MENU_ITEM(menu_item->widget)), -1); + } } static void on_break_delete(G_GNUC_UNUSED const MenuItem *menu_item) { GtkTreeIter iter; - gtk_tree_selection_get_selected(selection, NULL, &iter); - break_delete(&iter); + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) + { + break_delete(&iter); + } } static void break_seek_selected(gboolean focus) diff --git a/scope/src/inspect.c b/scope/src/inspect.c index e83f892f8..244457e46 100644 --- a/scope/src/inspect.c +++ b/scope/src/inspect.c @@ -815,25 +815,29 @@ static void on_inspect_edit(G_GNUC_UNUSED const MenuItem *menu_item) const char *name, *frame; gboolean run_apply; - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_get(store, &iter, INSPECT_EXPR, &expr, INSPECT_NAME, &name, - INSPECT_FRAME, &frame, INSPECT_RUN_APPLY, &run_apply, -1); - scp_tree_store_set(store, &iter, INSPECT_NAME, "-", -1); /* for duplicate name check */ - - gtk_entry_set_text(inspect_expr, expr); - gtk_entry_set_text(inspect_name, name); - gtk_entry_set_text(inspect_frame, frame); - gtk_toggle_button_set_active(inspect_run_apply, run_apply); - on_inspect_entry_changed(NULL, NULL); - - if (gtk_dialog_run(GTK_DIALOG(inspect_dialog)) == GTK_RESPONSE_ACCEPT) + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) { - g_free(jump_to_expr); - jump_to_expr = NULL; - inspect_dialog_store(&iter); + scp_tree_store_get(store, &iter, INSPECT_EXPR, &expr, INSPECT_NAME, &name, + INSPECT_FRAME, &frame, INSPECT_RUN_APPLY, &run_apply, -1); + scp_tree_store_set(store, &iter, INSPECT_NAME, "-", -1); /* for duplicate name check */ + + gtk_entry_set_text(inspect_expr, expr); + gtk_entry_set_text(inspect_name, name); + gtk_entry_set_text(inspect_frame, frame); + gtk_toggle_button_set_active(inspect_run_apply, run_apply); + on_inspect_entry_changed(NULL, NULL); + + if (gtk_dialog_run(GTK_DIALOG(inspect_dialog)) == GTK_RESPONSE_ACCEPT) + { + g_free(jump_to_expr); + jump_to_expr = NULL; + inspect_dialog_store(&iter); + } + else + { + scp_tree_store_set(store, &iter, INSPECT_NAME, name, -1); + } } - else - scp_tree_store_set(store, &iter, INSPECT_NAME, name, -1); } static void on_inspect_apply(G_GNUC_UNUSED const MenuItem *menu_item) @@ -842,13 +846,19 @@ static void on_inspect_apply(G_GNUC_UNUSED const MenuItem *menu_item) const char *var1; gint scid; - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_get(store, &iter, INSPECT_SCID, &scid, INSPECT_VAR1, &var1, -1); + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) + { + scp_tree_store_get(store, &iter, INSPECT_SCID, &scid, INSPECT_VAR1, &var1, -1); - if (var1) - debug_send_format(N, "070%d-var-delete %s", scid, var1); - else - inspect_apply(&iter); + if (var1) + { + debug_send_format(N, "070%d-var-delete %s", scid, var1); + } + else + { + inspect_apply(&iter); + } + } } static GtkWidget *expand_dialog; @@ -863,25 +873,31 @@ static void on_inspect_expand(G_GNUC_UNUSED const MenuItem *menu_item) gint start, count; gboolean expand; - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_get(store, &iter, INSPECT_NAME, &name, INSPECT_START, &start, - INSPECT_COUNT, &count, INSPECT_EXPAND, &expand, -1); - gtk_spin_button_set_value(expand_start, start); - gtk_spin_button_set_value(expand_count, count); - gtk_toggle_button_set_active(expand_automatic, expand); - gtk_widget_set_sensitive(GTK_WIDGET(expand_automatic), name != NULL); - - if (gtk_dialog_run(GTK_DIALOG(expand_dialog)) == GTK_RESPONSE_ACCEPT) + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) { - scp_tree_store_set(store, &iter, - INSPECT_START, gtk_spin_button_get_value_as_int(expand_start), - INSPECT_COUNT, gtk_spin_button_get_value_as_int(expand_count), - INSPECT_EXPAND, gtk_toggle_button_get_active(expand_automatic), -1); + scp_tree_store_get(store, &iter, INSPECT_NAME, &name, INSPECT_START, &start, + INSPECT_COUNT, &count, INSPECT_EXPAND, &expand, -1); + gtk_spin_button_set_value(expand_start, start); + gtk_spin_button_set_value(expand_count, count); + gtk_toggle_button_set_active(expand_automatic, expand); + gtk_widget_set_sensitive(GTK_WIDGET(expand_automatic), name != NULL); + + if (gtk_dialog_run(GTK_DIALOG(expand_dialog)) == GTK_RESPONSE_ACCEPT) + { + scp_tree_store_set(store, &iter, + INSPECT_START, gtk_spin_button_get_value_as_int(expand_start), + INSPECT_COUNT, gtk_spin_button_get_value_as_int(expand_count), + INSPECT_EXPAND, gtk_toggle_button_get_active(expand_automatic), -1); - if (debug_state() & DS_VARIABLE) - inspect_expand(&iter); - else - plugin_beep(); + if (debug_state() & DS_VARIABLE) + { + inspect_expand(&iter); + } + else + { + plugin_beep(); + } + } } } @@ -901,16 +917,20 @@ static void on_inspect_format_update(const MenuItem *menu_item) gint format = GPOINTER_TO_INT(menu_item->gdata); const char *var1; - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_get(store, &iter, INSPECT_VAR1, &var1, -1); - - if (var1) + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) { - debug_send_format(N, "07%d-var-set-format %s %s", inspect_get_scid(&iter), var1, - inspect_formats[format]); + scp_tree_store_get(store, &iter, INSPECT_VAR1, &var1, -1); + + if (var1) + { + debug_send_format(N, "07%d-var-set-format %s %s", inspect_get_scid(&iter), var1, + inspect_formats[format]); + } + else + { + scp_tree_store_set(store, &iter, INSPECT_FORMAT, format, -1); + } } - else - scp_tree_store_set(store, &iter, INSPECT_FORMAT, format, -1); } static void on_inspect_hbit_display(const MenuItem *menu_item) @@ -939,18 +959,20 @@ static void on_inspect_hbit_update(const MenuItem *menu_item) const char *expr, *name; gint hb_mode = GPOINTER_TO_INT(menu_item->gdata); - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_get(store, &iter, INSPECT_EXPR, &expr, INSPECT_NAME, &name, -1); - inspect_hbit_update_iter(&iter, hb_mode); - parse_mode_update(expr, MODE_HBIT, hb_mode); - - if (name) + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) { - char *reverse = parse_mode_reentry(expr); + scp_tree_store_get(store, &iter, INSPECT_EXPR, &expr, INSPECT_NAME, &name, -1); + inspect_hbit_update_iter(&iter, hb_mode); + parse_mode_update(expr, MODE_HBIT, hb_mode); + + if (name) + { + char *reverse = parse_mode_reentry(expr); - if (store_find(store, &iter, INSPECT_EXPR, reverse)) - inspect_hbit_update_iter(&iter, hb_mode); - g_free(reverse); + if (store_find(store, &iter, INSPECT_EXPR, reverse)) + inspect_hbit_update_iter(&iter, hb_mode); + g_free(reverse); + } } } @@ -959,13 +981,19 @@ static void on_inspect_delete(G_GNUC_UNUSED const MenuItem *menu_item) GtkTreeIter iter; const char *var1; - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_get(store, &iter, INSPECT_VAR1, &var1, -1); + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) + { + scp_tree_store_get(store, &iter, INSPECT_VAR1, &var1, -1); - if (var1) - debug_send_format(N, "071%d-var-delete %s", inspect_get_scid(&iter), var1); - else - scp_tree_store_remove(store, &iter); + if (var1) + { + debug_send_format(N, "071%d-var-delete %s", inspect_get_scid(&iter), var1); + } + else + { + scp_tree_store_remove(store, &iter); + } + } } #define DS_EDITABLE (DS_BASICS | DS_EXTRA_2) diff --git a/scope/src/local.c b/scope/src/local.c index 556b264a0..e86705f02 100644 --- a/scope/src/local.c +++ b/scope/src/local.c @@ -152,9 +152,11 @@ static void on_local_watch(G_GNUC_UNUSED const MenuItem *menu_item) GtkTreeIter iter; const char *name; - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_get(store, &iter, LOCAL_NAME, &name, -1); - watch_add(name); + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) + { + scp_tree_store_get(store, &iter, LOCAL_NAME, &name, -1); + watch_add(name); + } } static void on_local_inspect(G_GNUC_UNUSED const MenuItem *menu_item) diff --git a/scope/src/memory.c b/scope/src/memory.c index d128176ee..4b97ffd42 100644 --- a/scope/src/memory.c +++ b/scope/src/memory.c @@ -304,13 +304,15 @@ static void on_memory_copy(G_GNUC_UNUSED const MenuItem *menu_item) const gchar *ascii; gchar *string; - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_get(store, &iter, MEMORY_ADDR, &addr, MEMORY_BYTES, &bytes, - MEMORY_ASCII, &ascii, -1); - string = g_strdup_printf("%s%s%s", addr, bytes, ascii); - gtk_clipboard_set_text(gtk_widget_get_clipboard(menu_item->widget, - GDK_SELECTION_CLIPBOARD), string, -1); - g_free(string); + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) + { + scp_tree_store_get(store, &iter, MEMORY_ADDR, &addr, MEMORY_BYTES, &bytes, + MEMORY_ASCII, &ascii, -1); + string = g_strdup_printf("%s%s%s", addr, bytes, ascii); + gtk_clipboard_set_text(gtk_widget_get_clipboard(menu_item->widget, + GDK_SELECTION_CLIPBOARD), string, -1); + g_free(string); + } } static void on_memory_clear(G_GNUC_UNUSED const MenuItem *menu_item) diff --git a/scope/src/menu.c b/scope/src/menu.c index ac0123d4d..d7490927e 100644 --- a/scope/src/menu.c +++ b/scope/src/menu.c @@ -203,9 +203,11 @@ void menu_mode_display(GtkTreeSelection *selection, const MenuItem *menu_item, g GtkTreeIter iter; gint mode; - gtk_tree_selection_get_selected(selection, &model, &iter); - gtk_tree_model_get(model, &iter, column, &mode, -1); - menu_item_set_active(menu_item + mode + 1, TRUE); + if (gtk_tree_selection_get_selected(selection, &model, &iter)) + { + gtk_tree_model_get(model, &iter, column, &mode, -1); + menu_item_set_active(menu_item + mode + 1, TRUE); + } } static void menu_mode_update_iter(ScpTreeStore *store, GtkTreeIter *iter, gint new_mode, @@ -235,18 +237,20 @@ void menu_mode_update(GtkTreeSelection *selection, gint new_mode, gboolean hbit) GtkTreeIter iter; const char *name; - scp_tree_selection_get_selected(selection, &store, &iter); - scp_tree_store_get(store, &iter, COLUMN_NAME, &name, -1); - menu_mode_update_iter(store, &iter, new_mode, hbit); - parse_mode_update(name, hbit ? MODE_HBIT : MODE_MEMBER, new_mode); - - if (hbit) + if (scp_tree_selection_get_selected(selection, &store, &iter)) { - char *reverse = parse_mode_reentry(name); + scp_tree_store_get(store, &iter, COLUMN_NAME, &name, -1); + menu_mode_update_iter(store, &iter, new_mode, hbit); + parse_mode_update(name, hbit ? MODE_HBIT : MODE_MEMBER, new_mode); + + if (hbit) + { + char *reverse = parse_mode_reentry(name); - if (store_find(store, &iter, COLUMN_NAME, reverse)) - menu_mode_update_iter(store, &iter, new_mode, TRUE); - g_free(reverse); + if (store_find(store, &iter, COLUMN_NAME, reverse)) + menu_mode_update_iter(store, &iter, new_mode, TRUE); + g_free(reverse); + } } } @@ -306,18 +310,20 @@ void menu_copy(GtkTreeSelection *selection, const MenuItem *menu_item) const char *value; GString *string; - scp_tree_selection_get_selected(selection, &store, &iter); - scp_tree_store_get(store, &iter, COLUMN_NAME, &name, COLUMN_DISPLAY, &display, - COLUMN_VALUE, &value, -1); - string = g_string_new(name); + if (scp_tree_selection_get_selected(selection, &store, &iter)) + { + scp_tree_store_get(store, &iter, COLUMN_NAME, &name, COLUMN_DISPLAY, &display, + COLUMN_VALUE, &value, -1); + string = g_string_new(name); - if (value) - g_string_append_printf(string, " = %s", display); + if (value) + g_string_append_printf(string, " = %s", display); - gtk_clipboard_set_text(gtk_widget_get_clipboard(menu_item->widget, - GDK_SELECTION_CLIPBOARD), string->str, string->len); + gtk_clipboard_set_text(gtk_widget_get_clipboard(menu_item->widget, + GDK_SELECTION_CLIPBOARD), string->str, string->len); - g_string_free(string, TRUE); + g_string_free(string, TRUE); + } } static GtkWidget *modify_dialog; @@ -373,11 +379,13 @@ void menu_modify(GtkTreeSelection *selection, const MenuItem *menu_item) const char *value; gint hb_mode; - scp_tree_selection_get_selected(selection, &store, &iter); - scp_tree_store_get(store, &iter, COLUMN_NAME, &name, COLUMN_VALUE, &value, COLUMN_HB_MODE, - &hb_mode, -1); - menu_evaluate_modify(name, value, _("Modify"), hb_mode, menu_item ? MR_MODIFY : MR_MODSTR, - "07"); + if (scp_tree_selection_get_selected(selection, &store, &iter)) + { + scp_tree_store_get(store, &iter, COLUMN_NAME, &name, COLUMN_VALUE, &value, COLUMN_HB_MODE, + &hb_mode, -1); + menu_evaluate_modify(name, value, _("Modify"), hb_mode, menu_item ? MR_MODIFY : MR_MODSTR, + "07"); + } } void menu_inspect(GtkTreeSelection *selection) @@ -386,9 +394,11 @@ void menu_inspect(GtkTreeSelection *selection) GtkTreeIter iter; const char *name; - scp_tree_selection_get_selected(selection, &store, &iter); - scp_tree_store_get(store, &iter, COLUMN_NAME, &name, -1); - inspect_add(name); + if (scp_tree_selection_get_selected(selection, &store, &iter)) + { + scp_tree_store_get(store, &iter, COLUMN_NAME, &name, -1); + inspect_add(name); + } } void on_menu_display_booleans(const MenuItem *menu_item) diff --git a/scope/src/register.c b/scope/src/register.c index 193667a42..905b6b47d 100644 --- a/scope/src/register.c +++ b/scope/src/register.c @@ -459,16 +459,20 @@ static void on_register_format_update(const MenuItem *menu_item) gint format = GPOINTER_TO_INT(menu_item->gdata); gint id; - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_get(store, &iter, REGISTER_ID, &id, -1); - - if (debug_state() & DS_DEBUG) + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) { - debug_send_format(N, "02%d%c%s%s-data-list-register-values %c %d", format, - FRAME_ARGS, register_formats[format], id); + scp_tree_store_get(store, &iter, REGISTER_ID, &id, -1); + + if (debug_state() & DS_DEBUG) + { + debug_send_format(N, "02%d%c%s%s-data-list-register-values %c %d", format, + FRAME_ARGS, register_formats[format], id); + } + else + { + scp_tree_store_set(store, &iter, REGISTER_FORMAT, format, -1); + } } - else - scp_tree_store_set(store, &iter, REGISTER_FORMAT, format, -1); } static void on_register_query(G_GNUC_UNUSED const MenuItem *menu_item) diff --git a/scope/src/stack.c b/scope/src/stack.c index 124893a27..b03909feb 100644 --- a/scope/src/stack.c +++ b/scope/src/stack.c @@ -165,10 +165,12 @@ void on_stack_follow(GArray *nodes) gboolean stack_entry(void) { GtkTreeIter iter; - gboolean entry; + gboolean entry = NULL; - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_get(store, &iter, STACK_ENTRY, &entry, -1); + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) + { + scp_tree_store_get(store, &iter, STACK_ENTRY, &entry, -1); + } return entry; } @@ -268,18 +270,22 @@ static void on_stack_show_entry(const MenuItem *menu_item) GtkTreeIter iter; view_dirty(VIEW_LOCALS); - gtk_tree_selection_get_selected(selection, NULL, &iter); - scp_tree_store_get(store, &iter, STACK_FUNC, &ed.func, -1); - parse_mode_update(ed.func, MODE_ENTRY, ed.entry); - store_foreach(store, (GFunc) stack_iter_show_entry, &ed); - - if (ed.count == 1) + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) { - debug_send_format(T, "04%s-stack-list-arguments 1 %s %s", thread_id, frame_id, - frame_id); + scp_tree_store_get(store, &iter, STACK_FUNC, &ed.func, -1); + parse_mode_update(ed.func, MODE_ENTRY, ed.entry); + store_foreach(store, (GFunc) stack_iter_show_entry, &ed); + + if (ed.count == 1) + { + debug_send_format(T, "04%s-stack-list-arguments 1 %s %s", thread_id, frame_id, + frame_id); + } + else + { + debug_send_format(T, "04%s-stack-list-arguments 1", thread_id); + } } - else - debug_send_format(T, "04%s-stack-list-arguments 1", thread_id); } #define DS_VIEWABLE (DS_ACTIVE | DS_EXTRA_2) diff --git a/scope/src/store/scptreestore.c b/scope/src/store/scptreestore.c index 4ee0f4510..ee6d306d2 100644 --- a/scope/src/store/scptreestore.c +++ b/scope/src/store/scptreestore.c @@ -527,16 +527,19 @@ static void validate_store(ScpTreeStore *store) static gboolean scp_insert_element(ScpTreeStore *store, GtkTreeIter *iter, AElem *elem, gint position, GtkTreeIter *parent_iter) { - ScpTreeStorePrivate *priv = store->priv; - AElem *parent = parent_iter ? ITER_ELEM(parent_iter) : priv->root; - GPtrArray *array = parent->children; + ScpTreeStorePrivate *priv; + AElem *parent; + GPtrArray *array; GtkTreePath *path; g_return_val_if_fail(SCP_IS_TREE_STORE(store), FALSE); g_return_val_if_fail(iter != NULL, FALSE); + priv = store->priv; g_return_val_if_fail(priv->sublevels == TRUE || parent_iter == NULL, FALSE); g_return_val_if_fail(VALID_ITER_OR_NULL(parent_iter, store), FALSE); + parent = parent_iter ? ITER_ELEM(parent_iter) : priv->root; + array = parent->children; if (array) { if (position == -1) @@ -624,12 +627,13 @@ void scp_tree_store_insert_with_values(ScpTreeStore *store, GtkTreeIter *iter, void scp_tree_store_get_valist(ScpTreeStore *store, GtkTreeIter *iter, va_list ap) { ScpTreeStorePrivate *priv = store->priv; - AElem *elem = ITER_ELEM(iter); + AElem *elem; gint column; g_return_if_fail(SCP_IS_TREE_STORE(store)); g_return_if_fail(VALID_ITER(iter, store)); + elem = ITER_ELEM(iter); while ((column = va_arg(ap, int)) != -1) { gpointer dest; @@ -896,12 +900,13 @@ gboolean scp_tree_store_get_iter(ScpTreeStore *store, GtkTreeIter *iter, GtkTree GtkTreePath *scp_tree_store_get_path(VALIDATE_ONLY ScpTreeStore *store, GtkTreeIter *iter) { - AElem *elem = ITER_ELEM(iter); + AElem *elem; GtkTreePath *path; g_return_val_if_fail(VALID_ITER(iter, store), NULL); path = gtk_tree_path_new(); + elem = ITER_ELEM(iter); if (elem->parent) { gtk_tree_path_append_index(path, ITER_INDEX(iter)); diff --git a/scope/src/thread.c b/scope/src/thread.c index b038ec2b0..fd28cea46 100644 --- a/scope/src/thread.c +++ b/scope/src/thread.c @@ -775,14 +775,16 @@ static void on_thread_interrupt(G_GNUC_UNUSED const MenuItem *menu_item) GtkTreeIter iter; HANDLE hid; - gtk_tree_selection_get_selected(selection, NULL, &iter); - hid = iter_to_handle(&iter); - - if (hid) + if (gtk_tree_selection_get_selected(selection, NULL, &iter)) { - if (!DebugBreakProcess(hid)) - show_errno("DebugBreakProcess"); - CloseHandle(hid); + hid = iter_to_handle(&iter); + + if (hid) + { + if (!DebugBreakProcess(hid)) + show_errno("DebugBreakProcess"); + CloseHandle(hid); + } } }