Backport two use-after-free fixes from upstream

These should make gnome-software less crashy.
This commit is contained in:
Kalev Lember 2017-01-27 22:12:18 +01:00
parent 50ca224110
commit 4ac53f6bd8
3 changed files with 156 additions and 1 deletions

View File

@ -0,0 +1,43 @@
From c8777211378b38f6ac5fdc724965baad18d123b0 Mon Sep 17 00:00:00 2001
From: Simon McVittie <smcv@debian.org>
Date: Thu, 26 Jan 2017 20:27:33 +0000
Subject: [PATCH] AsStore: hold a ref in hash_unique_id to avoid use-after-free
(#149)
If we rely on the reference here being "borrowed" from priv->array,
then it's easy for that assumption to become unjustified. This
caused a crash for me in as_store_remove_app_by_id, which removes
the app from priv->array before it removes it from
priv->hash_unique_id; if no other code is holding a ref to the same
app, then as_app_get_unique_id() will be a use-after-free.
Signed-off-by: Simon McVittie <simon.mcvittie@collabora.co.uk>
---
libappstream-glib/as-store.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/libappstream-glib/as-store.c b/libappstream-glib/as-store.c
index e1e9ce8..84b6292 100644
--- a/libappstream-glib/as-store.c
+++ b/libappstream-glib/as-store.c
@@ -1217,7 +1217,7 @@ as_store_add_app (AsStore *store, AsApp *app)
g_ptr_array_add (priv->array, g_object_ref (app));
g_hash_table_insert (priv->hash_unique_id,
(gpointer) as_app_get_unique_id (app),
- app);
+ g_object_ref (app));
pkgnames = as_app_get_pkgnames (app);
for (i = 0; i < pkgnames->len; i++) {
pkgname = g_ptr_array_index (pkgnames, i);
@@ -3416,7 +3416,7 @@ as_store_init (AsStore *store)
priv->hash_unique_id = g_hash_table_new_full (g_str_hash,
g_str_equal,
NULL,
- NULL);
+ g_object_unref);
priv->hash_pkgname = g_hash_table_new_full (g_str_hash,
g_str_equal,
g_free,
--
2.9.3

View File

@ -0,0 +1,105 @@
From a3c03baee4b9ea9cb6ad8c4f04ab54c7fdec0a05 Mon Sep 17 00:00:00 2001
From: Kalev Lember <klember@redhat.com>
Date: Fri, 27 Jan 2017 15:11:43 +0100
Subject: [PATCH] store: Fix use after free with hash table keys
AsStore keeps a priv->hash_id hash table with app ID's as keys and a
GPtrArray with the actual apps matching the app ID as values. To optimize
memory use, the keys are all AsRefStrings. As all AsRefStrings are reference
counted, they may get freed when the last app with the matching ID gets
removed.
To handle that case, make sure the remove the GPtrArray holding the apps as
well when removing last app matching an ID.
This fixes the following use after free when starting gnome-software:
==6065== Invalid read of size 1
==6065== at 0x8E26650: __strcmp_sse2_unaligned (in /usr/lib64/libc-2.24.so)
==6065== by 0x8585678: g_str_equal (ghash.c:1846)
==6065== by 0x8584BDF: g_hash_table_lookup_node (ghash.c:396)
==6065== by 0x8584BDF: g_hash_table_lookup (ghash.c:1147)
==6065== by 0x4E789F2: as_store_add_app (as-store.c:1229)
==6065== by 0x4E797CA: as_store_from_root (as-store.c:1537)
==6065== by 0x4E7A71F: as_store_from_file_internal (as-store.c:1880)
==6065== by 0x4E7B7A9: as_store_load_app_info_file (as-store.c:2556)
==6065== by 0x4E7B9B8: as_store_load_app_info (as-store.c:2611)
==6065== by 0x4E7C352: as_store_search_app_info (as-store.c:2847)
==6065== by 0x4E7C598: as_store_search_per_system (as-store.c:2883)
==6065== by 0x4E7CF7F: as_store_load (as-store.c:3068)
==6065== by 0x1E512B7B: gs_plugin_setup (gs-plugin-appstream.c:255)
==6065== by 0x46A2C5: gs_plugin_loader_setup (gs-plugin-loader.c:3912)
==6065== by 0x46ECCE: gs_application_initialize_plugins (gs-application.c:228)
==6065== by 0x46ED99: gs_application_dbus_register (gs-application.c:249)
==6065== by 0x8007167: g_application_impl_attempt_primary (gapplicationimpl-dbus.c:406)
==6065== by 0x8007167: g_application_impl_register (gapplicationimpl-dbus.c:555)
==6065== by 0x8003F22: g_application_register (gapplication.c:2033)
==6065== by 0x47064D: gs_application_handle_local_options (gs-application.c:864)
==6065== by 0xEFCFC57: ffi_call_unix64 (unix64.S:76)
==6065== by 0xEFCF6B9: ffi_call (ffi64.c:525)
==6065== Address 0x259c1994 is 4 bytes inside a block of size 31 free'd
==6065== at 0x4C2ED4A: free (vg_replace_malloc.c:530)
==6065== by 0x859B6BD: g_free (gmem.c:189)
==6065== by 0x4E70A37: as_ref_string_unref_from_str (as-ref-string.c:48)
==6065== by 0x8583D22: g_hash_table_remove_node (ghash.c:455)
==6065== by 0x8584647: g_hash_table_remove_internal (ghash.c:1358)
==6065== by 0x4E70E09: as_ref_string_unref (as-ref-string.c:225)
==6065== by 0x4E510B4: as_app_finalize (as-app.c:473)
==6065== by 0x830E116: g_object_unref (gobject.c:3185)
==6065== by 0x8569E2D: g_ptr_array_remove_index (garray.c:1188)
==6065== by 0x856A230: g_ptr_array_remove (garray.c:1321)
==6065== by 0x4E77D49: as_store_remove_app (as-store.c:931)
==6065== by 0x4E789DB: as_store_add_app (as-store.c:1225)
==6065== by 0x4E797CA: as_store_from_root (as-store.c:1537)
==6065== by 0x4E7A71F: as_store_from_file_internal (as-store.c:1880)
==6065== by 0x4E7B7A9: as_store_load_app_info_file (as-store.c:2556)
==6065== by 0x4E7B9B8: as_store_load_app_info (as-store.c:2611)
==6065== by 0x4E7C352: as_store_search_app_info (as-store.c:2847)
==6065== by 0x4E7C598: as_store_search_per_system (as-store.c:2883)
==6065== by 0x4E7CF7F: as_store_load (as-store.c:3068)
==6065== by 0x1E512B7B: gs_plugin_setup (gs-plugin-appstream.c:255)
==6065== Block was alloc'd at
==6065== at 0x4C2DB9D: malloc (vg_replace_malloc.c:299)
==6065== by 0x859B5A8: g_malloc (gmem.c:94)
==6065== by 0x4E70ABF: as_ref_string_new_copy_with_length (as-ref-string.c:99)
==6065== by 0x4E6B908: as_node_reflow_text (as-node.c:516)
==6065== by 0x4E6BEF7: as_node_text_cb (as-node.c:707)
==6065== by 0x859A14D: g_markup_parse_context_parse (gmarkup.c:1514)
==6065== by 0x4E6C29C: as_node_from_xml (as-node.c:825)
==6065== by 0x4E5BAA8: as_app_parse_appdata_file (as-app.c:5665)
==6065== by 0x4E5BE6A: as_app_parse_file (as-app.c:5781)
==6065== by 0x4E7BEDB: as_store_load_installed (as-store.c:2743)
==6065== by 0x4E7C24B: as_store_search_installed (as-store.c:2825)
==6065== by 0x4E7C636: as_store_search_per_system (as-store.c:2890)
==6065== by 0x4E7CF7F: as_store_load (as-store.c:3068)
==6065== by 0x1E512B7B: gs_plugin_setup (gs-plugin-appstream.c:255)
==6065== by 0x46A2C5: gs_plugin_loader_setup (gs-plugin-loader.c:3912)
==6065== by 0x46ECCE: gs_application_initialize_plugins (gs-application.c:228)
==6065== by 0x46ED99: gs_application_dbus_register (gs-application.c:249)
==6065== by 0x8007167: g_application_impl_attempt_primary (gapplicationimpl-dbus.c:406)
==6065== by 0x8007167: g_application_impl_register (gapplicationimpl-dbus.c:555)
==6065== by 0x8003F22: g_application_register (gapplication.c:2033)
==6065== by 0x47064D: gs_application_handle_local_options (gs-application.c:864)
---
libappstream-glib/as-store.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/libappstream-glib/as-store.c b/libappstream-glib/as-store.c
index cdaaef8..b203797 100644
--- a/libappstream-glib/as-store.c
+++ b/libappstream-glib/as-store.c
@@ -927,6 +927,11 @@ as_store_remove_app (AsStore *store, AsApp *app)
if (apps != NULL)
g_ptr_array_remove (apps, app);
+ /* remove the array as well if it was the last app as the AsRefString
+ * with the app ID may get freed now */
+ if (apps->len == 0)
+ g_hash_table_remove (priv->hash_id, as_app_get_id (app));
+
g_hash_table_remove (priv->hash_unique_id, as_app_get_unique_id (app));
g_ptr_array_remove (priv->array, app);
g_hash_table_remove_all (priv->metadata_indexes);
--
2.9.3

View File

@ -6,13 +6,15 @@
Summary: Library for AppStream metadata
Name: libappstream-glib
Version: 0.6.7
Release: 2%{?dist}
Release: 3%{?dist}
License: LGPLv2+
URL: http://people.freedesktop.org/~hughsient/appstream-glib/
Source0: http://people.freedesktop.org/~hughsient/appstream-glib/releases/appstream-glib-%{version}.tar.xz
# Backported from upstream
Patch0: 0001-Tighten-a-rule-that-classifies-apps-as-web-apps.patch
Patch1: 0001-AsStore-hold-a-ref-in-hash_unique_id-to-avoid-use-af.patch
Patch2: 0001-store-Fix-use-after-free-with-hash-table-keys.patch
BuildRequires: glib2-devel >= %{glib2_version}
BuildRequires: libtool
@ -86,6 +88,8 @@ GLib headers and libraries for appstream-builder.
%prep
%setup -q -n appstream-glib-%{version}
%patch0 -p1
%patch1 -p1
%patch2 -p1
%build
%configure \
@ -154,6 +158,9 @@ make install DESTDIR=$RPM_BUILD_ROOT
%{_datadir}/gir-1.0/AppStreamBuilder-1.0.gir
%changelog
* Fri Jan 27 2017 Kalev Lember <klember@redhat.com> 0.6.7-3
- Backport two use-after-free fixes from upstream
* Mon Jan 16 2017 Kalev Lember <klember@redhat.com> 0.6.7-2
- Fix epiphany showing up twice in gnome-software