From 4ac53f6bd8995c5c5a5c94435101bb530312e87d Mon Sep 17 00:00:00 2001 From: Kalev Lember Date: Fri, 27 Jan 2017 22:12:18 +0100 Subject: [PATCH] Backport two use-after-free fixes from upstream These should make gnome-software less crashy. --- ...ef-in-hash_unique_id-to-avoid-use-af.patch | 43 +++++++ ...-use-after-free-with-hash-table-keys.patch | 105 ++++++++++++++++++ libappstream-glib.spec | 9 +- 3 files changed, 156 insertions(+), 1 deletion(-) create mode 100644 0001-AsStore-hold-a-ref-in-hash_unique_id-to-avoid-use-af.patch create mode 100644 0001-store-Fix-use-after-free-with-hash-table-keys.patch diff --git a/0001-AsStore-hold-a-ref-in-hash_unique_id-to-avoid-use-af.patch b/0001-AsStore-hold-a-ref-in-hash_unique_id-to-avoid-use-af.patch new file mode 100644 index 0000000..ab724b3 --- /dev/null +++ b/0001-AsStore-hold-a-ref-in-hash_unique_id-to-avoid-use-af.patch @@ -0,0 +1,43 @@ +From c8777211378b38f6ac5fdc724965baad18d123b0 Mon Sep 17 00:00:00 2001 +From: Simon McVittie +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 +--- + 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 + diff --git a/0001-store-Fix-use-after-free-with-hash-table-keys.patch b/0001-store-Fix-use-after-free-with-hash-table-keys.patch new file mode 100644 index 0000000..5bbd1e4 --- /dev/null +++ b/0001-store-Fix-use-after-free-with-hash-table-keys.patch @@ -0,0 +1,105 @@ +From a3c03baee4b9ea9cb6ad8c4f04ab54c7fdec0a05 Mon Sep 17 00:00:00 2001 +From: Kalev Lember +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 + diff --git a/libappstream-glib.spec b/libappstream-glib.spec index 6d684e9..bd6cf19 100644 --- a/libappstream-glib.spec +++ b/libappstream-glib.spec @@ -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 0.6.7-3 +- Backport two use-after-free fixes from upstream + * Mon Jan 16 2017 Kalev Lember 0.6.7-2 - Fix epiphany showing up twice in gnome-software