Subject: Re: [PATCH gnome-team v2 09/12] gnu: gtk+-2: Fix build by hardening list store.

From: Maxim Cournoyer <[email-masked]>
Date: Sat, 08 Apr 2023 15:47:07 -0400

Hi,

Liliana Marie Prikler <[email-masked]> writes:

> * gnu/packages/patches/gtk2-harden-list-store.patch: New file.
> * gnu/packages/gtk.scm (gtk+-2)[patches]: Add it here.
> * gnu/local.mk (dist_patch_DATA): Register it here.
> ---
>  gnu/local.mk                                  |  1 +
>  gnu/packages/gtk.scm                          |  1 +
>  .../patches/gtk2-harden-list-store.patch      | 42 +++++++++++++++++++
>  3 files changed, 44 insertions(+)
>  create mode 100644 gnu/packages/patches/gtk2-harden-list-store.patch
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 3e94281ccf..31456c5be8 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -1303,6 +1303,7 @@ dist_patch_DATA =						\
>    %D%/packages/patches/guile-rsvg-pkgconfig.patch		\
>    %D%/packages/patches/guile-emacs-fix-configure.patch		\
>    %D%/packages/patches/gtk2-fix-builder-test.patch		\
> +  %D%/packages/patches/gtk2-harden-list-store.patch		\
>    %D%/packages/patches/gtk2-respect-GUIX_GTK2_PATH.patch	\
>    %D%/packages/patches/gtk2-respect-GUIX_GTK2_IM_MODULE_FILE.patch \
>    %D%/packages/patches/gtk2-theme-paths.patch			\
> diff --git a/gnu/packages/gtk.scm b/gnu/packages/gtk.scm
> index c756f39e24..196d767160 100644
> --- a/gnu/packages/gtk.scm
> +++ b/gnu/packages/gtk.scm
> @@ -1014,6 +1014,7 @@ (define-public gtk+-2
>                  "1nn6kks1zyvb5xikr9y2k7r9bwjy1g4b0m0s66532bclymbwfamc"))
>                (patches (search-patches "gtk2-respect-GUIX_GTK2_PATH.patch"
>                                         "gtk2-respect-GUIX_GTK2_IM_MODULE_FILE.patch"
> +                                       "gtk2-harden-list-store.patch"
>                                         "gtk2-theme-paths.patch"
>                                         "gtk2-fix-builder-test.patch"))))
>      (build-system gnu-build-system)
> diff --git a/gnu/packages/patches/gtk2-harden-list-store.patch b/gnu/packages/patches/gtk2-harden-list-store.patch
> new file mode 100644
> index 0000000000..b107ba2bcc
> --- /dev/null
> +++ b/gnu/packages/patches/gtk2-harden-list-store.patch
> @@ -0,0 +1,42 @@
> +Mimic the implemenetation in gtk+-3.

Typo: implementation.

I'd like to see a bit more metadata in this patch; is it original work,
or was it retrieved from another distribution such as Debian?  It
probably exists elsewhere, if GTK2 can't be built anymore otherwise?

Does upstream still maintain GTK2, or is it completely abandoned?

> +
> +Index: gtk+-2.24.33/gtk/gtkliststore.c
> +===================================================================
> +--- gtk+-2.24.33.orig/gtk/gtkliststore.c
> ++++ gtk+-2.24.33/gtk/gtkliststore.c
> +@@ -1195,16 +1195,31 @@ gboolean
> + gtk_list_store_iter_is_valid (GtkListStore *list_store,
> +                               GtkTreeIter  *iter)
> + {
> ++  GSequenceIter *seq_iter;
> ++
> +   g_return_val_if_fail (GTK_IS_LIST_STORE (list_store), FALSE);
> +   g_return_val_if_fail (iter != NULL, FALSE);
> + 
> +-  if (!VALID_ITER (iter, list_store))
> +-    return FALSE;
> ++  /* can't use VALID_ITER() here, because iter might point
> ++   * to random memory.
> ++   *
> ++   * We MUST NOT dereference it.
> ++   */
> + 
> +-  if (g_sequence_iter_get_sequence (iter->user_data) != list_store->seq)
> ++  if (iter == NULL ||
> ++      iter->user_data == NULL ||
> ++      list_store->stamp != iter->stamp)
> +     return FALSE;
> + 
> +-  return TRUE;
> ++  for (seq_iter = g_sequence_get_begin_iter (list_store->seq);
> ++       !g_sequence_iter_is_end (seq_iter);
> ++       seq_iter = g_sequence_iter_next (seq_iter))
> ++    {
> ++      if (seq_iter == iter->user_data)
> ++        return TRUE;
> ++    }
> ++
> ++  return FALSE;
> + }
> + 
> + static gboolean real_gtk_list_store_row_draggable (GtkTreeDragSource *drag_source,

I don't know my way much in this code base, but the above looks
reasonable to me, especially if it was mostly copy-pasted from GTK 3.

-- 
Thanks,
Maxim