From 58c111843175f5693c340362544ad34c69fe448587f0abd1133ddfe3de4544eb Mon Sep 17 00:00:00 2001 From: Christophe Giboudeaux Date: Tue, 30 Jul 2019 08:48:39 +0000 Subject: [PATCH] Accepting request 719783 from home:Vogtinator:boo1139463 - Add patches to fix crash with some invalid metainfo (boo#1139463): * 0001-Fix-possible-NULL-dereference.patch * 0002-Don-t-ignore-xmlNodeDump-return-code.patch * 0003-Fix-infinite-recursion-if-component-has-itself-liste.patch - Update build requirements - Add %check section OBS-URL: https://build.opensuse.org/request/show/719783 OBS-URL: https://build.opensuse.org/package/show/KDE:Frameworks5/AppStream?expand=0&rev=50 --- 0001-Fix-possible-NULL-dereference.patch | 29 ++++++++ ...Don-t-ignore-xmlNodeDump-return-code.patch | 44 ++++++++++++ ...ursion-if-component-has-itself-liste.patch | 72 +++++++++++++++++++ AppStream.changes | 10 +++ AppStream.spec | 16 ++--- 5 files changed, 161 insertions(+), 10 deletions(-) create mode 100644 0001-Fix-possible-NULL-dereference.patch create mode 100644 0002-Don-t-ignore-xmlNodeDump-return-code.patch create mode 100644 0003-Fix-infinite-recursion-if-component-has-itself-liste.patch diff --git a/0001-Fix-possible-NULL-dereference.patch b/0001-Fix-possible-NULL-dereference.patch new file mode 100644 index 0000000..fc02cfd --- /dev/null +++ b/0001-Fix-possible-NULL-dereference.patch @@ -0,0 +1,29 @@ +From 9dbdb8257e95a1f657dc043028a354ac17091875 Mon Sep 17 00:00:00 2001 +From: Matthias Klumpp +Date: Sat, 13 Jul 2019 16:03:47 +0200 +Subject: [PATCH 1/3] Fix possible NULL dereference + +An error check was missing here. Thanks Coverity for pointing this out! +--- + src/as-cache.c | 5 +++++ + 1 file changed, 5 insertions(+) + +diff --git a/src/as-cache.c b/src/as-cache.c +index 41f78a8b..50b42193 100644 +--- a/src/as-cache.c ++++ b/src/as-cache.c +@@ -1589,6 +1589,11 @@ as_cache_register_addons_for_component (AsCache *cache, MDB_txn *txn, AsComponen + return TRUE; + + addons = as_cache_components_by_hash_list (cache, txn, dval.mv_data, dval.mv_size, &tmp_error); ++ if (addons == NULL) { ++ g_propagate_error (error, tmp_error); ++ return FALSE; ++ } ++ + for (guint i = 0; i < addons->len; i++) + as_component_add_addon (cpt, AS_COMPONENT (g_ptr_array_index (addons, i))); + +-- +2.22.0 + diff --git a/0002-Don-t-ignore-xmlNodeDump-return-code.patch b/0002-Don-t-ignore-xmlNodeDump-return-code.patch new file mode 100644 index 0000000..ff9fc74 --- /dev/null +++ b/0002-Don-t-ignore-xmlNodeDump-return-code.patch @@ -0,0 +1,44 @@ +From 993ea2bc6917327f3f4de421cd8f9594f550ff98 Mon Sep 17 00:00:00 2001 +From: Matthias Klumpp +Date: Tue, 30 Jul 2019 02:14:53 +0200 +Subject: [PATCH 2/3] Don't ignore xmlNodeDump return code + +This should not fail, ever, unless we run out of memory. But since I was +looking at that code, having a sanity check here is better in case this +does become more relevant in future (and simply because not checking it +was not good prectice). +--- + src/as-xml.c | 13 ++++++++++--- + 1 file changed, 10 insertions(+), 3 deletions(-) + +diff --git a/src/as-xml.c b/src/as-xml.c +index 2ba64743..bfa90e72 100644 +--- a/src/as-xml.c ++++ b/src/as-xml.c +@@ -94,13 +94,20 @@ as_xml_dump_node_children (xmlNode *node) + + str = g_string_new (""); + for (iter = node->children; iter != NULL; iter = iter->next) { ++ gint r; ++ + /* discard spaces */ + if (iter->type != XML_ELEMENT_NODE) { +- continue; ++ continue; + } + +- nodeBuf = xmlBufferCreate(); +- xmlNodeDump (nodeBuf, NULL, iter, 0, 1); ++ nodeBuf = xmlBufferCreate (); ++ r = xmlNodeDump (nodeBuf, NULL, iter, 0, 1); ++ if (r < 0) { ++ xmlBufferFree (nodeBuf); ++ g_warning ("xmlNodeDump failed (%i) while serializing node children.", r); ++ continue; ++ } + if (str->len > 0) + g_string_append (str, "\n"); + g_string_append_printf (str, "%s", (const gchar*) nodeBuf->content); +-- +2.22.0 + diff --git a/0003-Fix-infinite-recursion-if-component-has-itself-liste.patch b/0003-Fix-infinite-recursion-if-component-has-itself-liste.patch new file mode 100644 index 0000000..ce88cbc --- /dev/null +++ b/0003-Fix-infinite-recursion-if-component-has-itself-liste.patch @@ -0,0 +1,72 @@ +From 823d7065ffcaec57bdbef479dce49ae97ff08640 Mon Sep 17 00:00:00 2001 +From: Matthias Klumpp +Date: Tue, 30 Jul 2019 02:38:47 +0200 +Subject: [PATCH 3/3] Fix infinite recursion if component has itself listed as + an addon + +This particular case of a component being an addon to itself is +nonsense, but people may make that mistake and we shouldn't crash in +that case. +With this patch the cache will be resilient against such cases and +simply ignore components depending on themselves. +We could still get nasty dependency loops though, with A depending on B +depending on A. This is a bit more complicated to resolve and will be +fixed in a future commit. +Resolves: #243 +--- + src/as-cache.c | 31 +++++++++++++++++++++++-------- + 1 file changed, 23 insertions(+), 8 deletions(-) + +diff --git a/src/as-cache.c b/src/as-cache.c +index 50b42193..3afabda7 100644 +--- a/src/as-cache.c ++++ b/src/as-cache.c +@@ -1573,7 +1573,7 @@ as_cache_register_addons_for_component (AsCache *cache, MDB_txn *txn, AsComponen + { + AsCachePrivate *priv = GET_PRIVATE (cache); + MDB_val dval; +- g_autoptr(GPtrArray) addons = NULL; ++ g_autofree guint8 *cpt_checksum = NULL; + GError *tmp_error = NULL; + + dval = as_cache_txn_get_value (cache, +@@ -1588,14 +1588,29 @@ as_cache_register_addons_for_component (AsCache *cache, MDB_txn *txn, AsComponen + if (dval.mv_size == 0) + return TRUE; + +- addons = as_cache_components_by_hash_list (cache, txn, dval.mv_data, dval.mv_size, &tmp_error); +- if (addons == NULL) { +- g_propagate_error (error, tmp_error); +- return FALSE; +- } ++ /* retrieve cache checksum of this component */ ++ as_generate_cache_checksum (as_component_get_data_id (cpt), ++ -1, ++ &cpt_checksum, ++ NULL); ++ ++ g_assert_cmpint (dval.mv_size % AS_CACHE_CHECKSUM_LEN, ==, 0); ++ for (gsize i = 0; i < dval.mv_size; i += AS_CACHE_CHECKSUM_LEN) { ++ const guint8 *chash = dval.mv_data + i; ++ AsComponent *addon; ++ ++ /* ignore addon that extends itself to prevent infinite recursion */ ++ if (memcmp (chash, cpt_checksum, AS_CACHE_CHECKSUM_LEN) == 0) ++ continue; + +- for (guint i = 0; i < addons->len; i++) +- as_component_add_addon (cpt, AS_COMPONENT (g_ptr_array_index (addons, i))); ++ addon = as_cache_component_by_hash (cache, txn, chash, &tmp_error); ++ if (tmp_error != NULL) { ++ g_propagate_prefixed_error (error, tmp_error, "Failed to retrieve addon component data: "); ++ return FALSE; ++ } ++ if (addon != NULL) ++ as_component_add_addon (cpt, addon); ++ } + + return TRUE; + } +-- +2.22.0 + diff --git a/AppStream.changes b/AppStream.changes index cd89d81..b6ee0d6 100644 --- a/AppStream.changes +++ b/AppStream.changes @@ -1,3 +1,13 @@ +------------------------------------------------------------------- +Tue Jul 30 07:56:11 UTC 2019 - Fabian Vogt + +- Add patches to fix crash with some invalid metainfo (boo#1139463): + * 0001-Fix-possible-NULL-dereference.patch + * 0002-Don-t-ignore-xmlNodeDump-return-code.patch + * 0003-Fix-infinite-recursion-if-component-has-itself-liste.patch +- Update build requirements +- Add %check section + ------------------------------------------------------------------- Mon Jul 29 18:52:40 UTC 2019 - Fabian Vogt diff --git a/AppStream.spec b/AppStream.spec index 6781dd7..702e1a7 100644 --- a/AppStream.spec +++ b/AppStream.spec @@ -30,11 +30,13 @@ Source1: http://www.freedesktop.org/software/appstream/releases/%{name}-% Source2: %{name}.keyring # PATCH-FIX-UPSTREAM Patch1: 0001-Restore-compatibility-with-GLib-2.58.patch +Patch2: 0001-Fix-possible-NULL-dereference.patch +Patch3: 0002-Don-t-ignore-xmlNodeDump-return-code.patch +Patch4: 0003-Fix-infinite-recursion-if-component-has-itself-liste.patch # PATCH-FIX-UPSTREAM (https://github.com/ximion/appstream/issues/239) Patch1000: find-lmdb.patch BuildRequires: gettext BuildRequires: gperf -BuildRequires: intltool BuildRequires: itstool BuildRequires: lmdb-devel BuildRequires: meson >= 0.42 @@ -46,10 +48,7 @@ BuildRequires: pkgconfig(gio-2.0) BuildRequires: pkgconfig(glib-2.0) >= 2.46 BuildRequires: pkgconfig(gobject-introspection-1.0) BuildRequires: pkgconfig(libxml-2.0) -BuildRequires: pkgconfig(packagekit-glib2) -BuildRequires: pkgconfig(protobuf) BuildRequires: pkgconfig(vapigen) -BuildRequires: pkgconfig(xapian-core) BuildRequires: pkgconfig(yaml-0.1) Recommends: curl @@ -121,12 +120,6 @@ GObject introspection bindings for interfaces provided by AppStream. %autosetup -p1 %build -%if "%{?_lib}" == "lib64" -SUFFIX="64" -%else -SUFFIX="" -%endif - %meson -Dqt=true \ -Dvapi=true \ -Ddocs=false \ @@ -137,6 +130,9 @@ SUFFIX="" %install %meson_install +%check +%meson_test + %find_lang appstream %{name}.lang %post