Fix an invalid read when using as_app_parse_data() from Python

Using GObject Introspection we were creating an object using GLib.Bytes.new(buf)
where buf was a python str object. This created a GBytes object with no
trailing NUL char but we were expecting a NUL-terminated string when both doing
fnmatch() and also processing the XML. Support this by guarding fnmatch and
also by using the string length when using the GMarkupParseContext.

4 files changed, 79 insertions, 22 deletions

diff --git a/libappstream-glib/as-app.c b/libappstream-glib/as-app.c
index bdf4d24..474af29 100644
--- a/libappstream-glib/as-app.c
+++ b/libappstream-glib/as-app.c
@@ -6047,6 +6047,16 @@ as_app_parse_appdata_guess_project_group (AsApp *app)
 	}
 }
 
+static int
+as_utils_fnmatch (const gchar *pattern, const gchar *text, gsize text_sz, gint flags)
+{
+	if (text_sz != -1 && text[text_sz-1] != '\0') {
+		g_autofree gchar *text_with_nul = g_strndup (text, text_sz);
+		return fnmatch (pattern, text_with_nul, flags);
+	}
+	return fnmatch (pattern, text, flags);
+}
+
 /**
  * as_app_parse_data:
  * @app: a #AsApp instance.
@@ -6078,13 +6088,13 @@ as_app_parse_data (AsApp *app, GBytes *data, guint32 flags, GError **error)
 		priv->problems |= AS_APP_PROBLEM_NO_XML_HEADER;
 
 	/* check for copyright */
-	if (fnmatch ("*<!--*Copyright*-->*", data_raw, 0) != 0)
+	if (as_utils_fnmatch ("*<!--*Copyright*-->*", data_raw, len, 0) != 0)
 		priv->problems |= AS_APP_PROBLEM_NO_COPYRIGHT_INFO;
 
 	/* parse */
 	if (flags & AS_APP_PARSE_FLAG_KEEP_COMMENTS)
 		from_xml_flags |= AS_NODE_FROM_XML_FLAG_KEEP_COMMENTS;
-	root = as_node_from_xml (data_raw, from_xml_flags, error);
+	root = as_node_from_bytes (data, from_xml_flags, error);
 	if (root == NULL)
 		return FALSE;
 
diff --git a/libappstream-glib/as-node.c b/libappstream-glib/as-node.c
index 36b4ae1..f8199a9 100644
--- a/libappstream-glib/as-node.c
+++ b/libappstream-glib/as-node.c
@@ -783,22 +783,10 @@ as_node_passthrough_cb (GMarkupParseContext *context,
 	}
 }
 
-/**
- * as_node_from_xml: (skip)
- * @data: XML data
- * @flags: #AsNodeFromXmlFlags, e.g. %AS_NODE_FROM_XML_FLAG_NONE
- * @error: A #GError or %NULL
- *
- * Parses XML data into a DOM tree.
- *
- * Returns: (transfer none): A populated #AsNode tree
- *
- * Since: 0.1.0
- **/
-AsNode *
-as_node_from_xml (const gchar *data,
-		  AsNodeFromXmlFlags flags,
-		  GError **error)
+static AsNode *
+as_node_from_xml_internal (const gchar *data, gssize data_sz,
+			   AsNodeFromXmlFlags flags,
+			   GError **error)
 {
 	AsNodeToXmlHelper helper;
 	AsNode *root = NULL;
@@ -822,10 +810,7 @@ as_node_from_xml (const gchar *data,
 					  G_MARKUP_PREFIX_ERROR_POSITION,
 					  &helper,
 					  NULL);
-	ret = g_markup_parse_context_parse (ctx,
-					    data,
-					    -1,
-					    &error_local);
+	ret = g_markup_parse_context_parse (ctx, data, data_sz, &error_local);
 	if (!ret) {
 		g_set_error_literal (error,
 				     AS_NODE_ERROR,
@@ -848,6 +833,46 @@ as_node_from_xml (const gchar *data,
 }
 
 /**
+ * as_node_from_bytes: (skip)
+ * @bytes: a #GBytes
+ * @flags: #AsNodeFromXmlFlags, e.g. %AS_NODE_FROM_XML_FLAG_NONE
+ * @error: A #GError or %NULL
+ *
+ * Parses XML data into a DOM tree.
+ *
+ * Returns: (transfer none): A populated #AsNode tree
+ *
+ * Since: 0.7.6
+ **/
+AsNode *
+as_node_from_bytes (GBytes *bytes, AsNodeFromXmlFlags flags, GError **error)
+{
+	gsize sz = 0;
+	const gchar *buf;
+	g_return_val_if_fail (bytes != NULL, NULL);
+	buf = g_bytes_get_data (bytes, &sz);
+	return as_node_from_xml_internal (buf, (gssize) sz, flags, error);
+}
+
+/**
+ * as_node_from_xml: (skip)
+ * @data: XML data
+ * @flags: #AsNodeFromXmlFlags, e.g. %AS_NODE_FROM_XML_FLAG_NONE
+ * @error: A #GError or %NULL
+ *
+ * Parses XML data into a DOM tree.
+ *
+ * Returns: (transfer none): A populated #AsNode tree
+ *
+ * Since: 0.1.0
+ **/
+AsNode *
+as_node_from_xml (const gchar *data, AsNodeFromXmlFlags flags, GError **error)
+{
+	return as_node_from_xml_internal (data, -1, flags, error);
+}
+
+/**
  * as_node_to_file: (skip)
  * @root: A populated #AsNode tree
  * @file: a #GFile
diff --git a/libappstream-glib/as-node.h b/libappstream-glib/as-node.h
index e3fe936..913c1c8 100644
--- a/libappstream-glib/as-node.h
+++ b/libappstream-glib/as-node.h
@@ -164,6 +164,10 @@ GNode		*as_node_from_xml		(const gchar	*data,
 						 AsNodeFromXmlFlags flags,
 						 GError		**error)
 						 G_GNUC_WARN_UNUSED_RESULT;
+GNode		*as_node_from_bytes		(GBytes		*bytes,
+						 AsNodeFromXmlFlags flags,
+						 GError		**error)
+						 G_GNUC_WARN_UNUSED_RESULT;
 GNode		*as_node_from_file		(GFile		*file,
 						 AsNodeFromXmlFlags flags,
 						 GCancellable	*cancellable,
diff --git a/libappstream-glib/as-self-test.c b/libappstream-glib/as-self-test.c
index 7b3b6df..cde6fda 100644
--- a/libappstream-glib/as-self-test.c
+++ b/libappstream-glib/as-self-test.c
@@ -24,6 +24,7 @@
 #include <glib.h>
 #include <glib/gstdio.h>
 #include <stdlib.h>
+#include <string.h>
 #include <fnmatch.h>
 
 #include "as-app-private.h"
@@ -5473,6 +5474,22 @@ as_test_utils_unique_id_hash_safe_func (void)
 }
 
 static void
+as_test_app_parse_data_func (void)
+{
+	const gchar *data = "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+			    "<component>\n</component>\n     ";
+	gboolean ret;
+	g_autoptr(GBytes) blob = NULL;
+	g_autoptr(AsApp) app = as_app_new ();
+	g_autoptr(GError) error = NULL;
+
+	blob = g_bytes_new (data, strlen (data));
+	ret = as_app_parse_data (app, blob, AS_APP_PARSE_FLAG_NONE, &error);
+	g_assert_no_error (error);
+	g_assert (ret);
+}
+
+static void
 as_test_ref_string_func (void)
 {
 	const gchar *tmp;
@@ -5546,6 +5563,7 @@ main (int argc, char **argv)
 	g_test_add_func ("/AppStream/app{validate-file-bad}", as_test_app_validate_file_bad_func);
 	g_test_add_func ("/AppStream/app{validate-meta-bad}", as_test_app_validate_meta_bad_func);
 	g_test_add_func ("/AppStream/app{validate-intltool}", as_test_app_validate_intltool_func);
+	g_test_add_func ("/AppStream/app{parse-data}", as_test_app_parse_data_func);
 	g_test_add_func ("/AppStream/app{parse-file:desktop}", as_test_app_parse_file_desktop_func);
 	g_test_add_func ("/AppStream/app{no-markup}", as_test_app_no_markup_func);
 	g_test_add_func ("/AppStream/app{subsume}", as_test_app_subsume_func);