talons

commit c8813ed987cc450627ab73bd8bf7838dfd710bbc
parent dca37632f4c6775be95392cd802c58e3ea39c175
Author: Andrej Kacian <ticho@claws-mail.org>
Date:   Wed,  3 Oct 2018 18:27:39 +0200

Make entity_decode() handle invalid input better.

Diffstat:
M
src/entity.c
 | 
28
+++++++++++++++++++++++++---
M
src/tests/entity_test.c
 | 
70
+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++---

2 files changed, 92 insertions(+), 6 deletions(-)
diff --git a/src/entity.c b/src/entity.c
@@ -337,7 +337,8 @@ static gchar *entity_decode_numeric(gchar *str)
 	gchar b[ENTITY_MAX_LEN];
 	gchar *p = str, *res;
 	gboolean hex = FALSE;
-	gunichar c;
+	gunichar c = -1;
+	gint ret;
 
 	++p;
 	if (*p == '\0')
@@ -353,9 +354,30 @@ static gchar *entity_decode_numeric(gchar *str)
 	if (entity_extract_to_buffer (p, b) == NULL)
 		return NULL;
 
-	c = g_ascii_strtoll (b, NULL, (hex? 16: 10));
+	if (strlen(b) > 0)
+		c = g_ascii_strtoll (b, NULL, (hex ? 16 : 10));
+
+	if (c < 0) {
+		/* Obviously invalid */
+		debug_print("Numeric reference '&#%s;' is invalid\n", b);
+		return NULL;
+	} else if (c >= 0 && c <= 31) {
+		/* An unprintable character; return the Unicode replacement symbol */
+		return g_strdup("\xef\xbf\xbd");
+	} else if (c > 0x10ffff) {
+		/* Make sure the character falls within the Unicode codespace
+		 * (0x0 - 0x10ffff) */
+		debug_print("Numeric reference '&#%s;' is invalid, outside of Unicode codespace\n", b);
+		return NULL;
+	}
+
 	res = g_malloc0 (DECODED_MAX_LEN + 1);
-	g_unichar_to_utf8 (c, res);
+	ret = g_unichar_to_utf8 (c, res);
+	if (ret == 0) {
+		debug_print("Failed to convert unicode character %u to UTF-8\n", c);
+		g_free(res);
+		res = NULL;
+	}
 
 	return res;
 }
diff --git a/src/tests/entity_test.c b/src/tests/entity_test.c
@@ -1,18 +1,78 @@
 #include <glib.h>
+#include <stdio.h>
+
 #include "mock_debug_print.h"
 
 #include "entity.h"
 
 static void
-test_entity(void)
+test_entity_invalid(void)
 {
 	gchar *result;
 
+	/* Invalid entity strings */
 	result = entity_decode(NULL);
 	g_assert_null(result);
-
+	result = entity_decode("");
+	g_assert_null(result);
 	result = entity_decode("foo");
 	g_assert_null(result);
+	result = entity_decode("&");
+	g_assert_null(result);
+	result = entity_decode("&;");
+	g_assert_null(result);
+	result = entity_decode("&#");
+	g_assert_null(result);
+	result = entity_decode("&#;");
+	g_assert_null(result);
+
+	/* Valid entity string, but with missing semicolon */
+	result = entity_decode("&Aacute");
+	g_assert_null(result);
+	result = entity_decode("&#123");
+	g_assert_null(result);
+}
+
+static void
+test_entity_toolong(void)
+{
+	gchar *result;
+
+	/* Largest unicode code point is 0x10ffff, let's test that,
+	 * and one past it */
+	result = entity_decode("&#1114111;");
+	g_assert_nonnull(result);
+	g_free(result);
+	result = entity_decode("&#1114112;");
+	g_assert_null(result);
+
+	/* ENTITY_MAX_LEN is 8, test 8- and 9-char entity strings
+	 * for possible buffer overflows */
+	result = entity_decode("&#88888888;");
+	g_assert_null(result);
+	result = entity_decode("&#999999999;");
+	g_assert_null(result);
+}
+
+static void
+test_entity_unprintable(void)
+{
+	gchar *result, numstr[6]; /* "&#XX;" */
+	gint i;
+
+	for (i = 0; i < 32; i++) {
+		sprintf(numstr, "&#%d;", i);
+		result = entity_decode(numstr);
+		g_assert_nonnull(result);
+		g_assert_cmpstr(result, ==, "\xef\xbf\xbd");
+		g_free(result);
+	}
+}
+
+static void
+test_entity_valid(void)
+{
+	gchar *result;
 
 	result = entity_decode("&Aacute;");
 	g_assert_nonnull(result);
@@ -27,6 +87,7 @@ test_entity(void)
 		g_printerr("result '%s'\n", result);
 	g_assert_cmpstr(result, ==, "{");
 	g_free(result);
+
 }
 
 int
@@ -34,7 +95,10 @@ main(int argc, char *argv[])
 {
 	g_test_init(&argc, &argv, NULL);
 
-	g_test_add_func("/core/entity", test_entity);
+	g_test_add_func("/core/entity/invalid", test_entity_invalid);
+	g_test_add_func("/core/entity/toolong", test_entity_toolong);
+	g_test_add_func("/core/entity/unprintable", test_entity_unprintable);
+	g_test_add_func("/core/entity/valid", test_entity_valid);
 
 	return g_test_run();
 }