From 19d55caab71e8885ada275241ef191526761f37a Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 5 Oct 2020 13:28:44 +0100 Subject: [PATCH 1/3] tests: Refactor g_uri_escape_string() tests This will allow more tests to be added easily in future. It introduces no functional changes. Signed-off-by: Philip Withnall --- glib/tests/uri.c | 53 ++++++++++++++++++++++++++++++++++-------------- 1 file changed, 38 insertions(+), 15 deletions(-) diff --git a/glib/tests/uri.c b/glib/tests/uri.c index 6de4c9cdc..31bef2733 100644 --- a/glib/tests/uri.c +++ b/glib/tests/uri.c @@ -449,22 +449,44 @@ test_uri_unescape_segment (void) } static void -test_uri_escape (void) +test_uri_escape_string (void) { - gchar *s; + const struct + { + /* Inputs */ + const gchar *unescaped; + const gchar *reserved_chars_allowed; + gboolean allow_utf8; + /* Outputs */ + const gchar *expected_escaped; + } + tests[] = + { + { "abcdefgABCDEFG._~", NULL, FALSE, "abcdefgABCDEFG._~" }, + { ":+ \\?#", NULL, FALSE, "%3A%2B%20%5C%3F%23" }, + { "a+b:c", "+", FALSE, "a+b%3Ac" }, + { "a+b:c\303\234", "+", TRUE, "a+b%3Ac\303\234" }, + }; + gsize i; - s = g_uri_escape_string ("abcdefgABCDEFG._~", NULL, FALSE); - g_assert_cmpstr (s, ==, "abcdefgABCDEFG._~"); - g_free (s); - s = g_uri_escape_string (":+ \\?#", NULL, FALSE); - g_assert_cmpstr (s, ==, "%3A%2B%20%5C%3F%23"); - g_free (s); - s = g_uri_escape_string ("a+b:c", "+", FALSE); - g_assert_cmpstr (s, ==, "a+b%3Ac"); - g_free (s); - s = g_uri_escape_string ("a+b:c\303\234", "+", TRUE); - g_assert_cmpstr (s, ==, "a+b%3Ac\303\234"); - g_free (s); + for (i = 0; i < G_N_ELEMENTS (tests); i++) + { + gchar *s = NULL; + + g_test_message ("Test %" G_GSIZE_FORMAT ": %s", i, tests[i].unescaped); + + s = g_uri_escape_string (tests[i].unescaped, + tests[i].reserved_chars_allowed, + tests[i].allow_utf8); + g_assert_cmpstr (s, ==, tests[i].expected_escaped); + g_free (s); + } +} + +static void +test_uri_escape_bytes (void) +{ + gchar *s = NULL; s = g_uri_escape_bytes ((guchar*)"\0\0", 2, NULL); g_assert_cmpstr (s, ==, "%00%00"); @@ -1688,7 +1710,8 @@ main (int argc, g_test_add_data_func ("/uri/unescape-bytes/nul-terminated", GINT_TO_POINTER (TRUE), test_uri_unescape_bytes); g_test_add_data_func ("/uri/unescape-bytes/length", GINT_TO_POINTER (FALSE), test_uri_unescape_bytes); g_test_add_func ("/uri/unescape-segment", test_uri_unescape_segment); - g_test_add_func ("/uri/escape", test_uri_escape); + g_test_add_func ("/uri/escape-string", test_uri_escape_string); + g_test_add_func ("/uri/escape-bytes", test_uri_escape_bytes); g_test_add_func ("/uri/scheme", test_uri_scheme); g_test_add_func ("/uri/parsing/absolute", test_uri_parsing_absolute); g_test_add_func ("/uri/parsing/relative", test_uri_parsing_relative); From 439b29087849b4533c4a76f5ddd7a8a29c007ae9 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 5 Oct 2020 12:10:40 +0100 Subject: [PATCH 2/3] guri: Fix UTF-8 validation when escaping URI components The return value from `g_utf8_get_char_validated()` is a `gunichar`, which is unsigned, so comparing it with `> 0` is always going to return true, even for return values `(gunichar) -1` and `(gunichar) -2`, which indicate errors. Handle them more explicitly. oss-fuzz#26083 Signed-off-by: Philip Withnall --- glib/guri.c | 9 +++++++-- glib/tests/uri.c | 4 ++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/glib/guri.c b/glib/guri.c index e337c9e24..f04139b80 100644 --- a/glib/guri.c +++ b/glib/guri.c @@ -420,8 +420,13 @@ _uri_encoder (GString *out, while (p < end) { - if (allow_utf8 && *p >= 0x80 && - g_utf8_get_char_validated ((gchar *)p, end - p) > 0) + gunichar multibyte_utf8_char = 0; + + if (allow_utf8 && *p >= 0x80) + multibyte_utf8_char = g_utf8_get_char_validated ((gchar *)p, end - p); + + if (multibyte_utf8_char > 0 && + multibyte_utf8_char != (gunichar) -1 && multibyte_utf8_char != (gunichar) -2) { gint len = g_utf8_skip [*p]; g_string_append_len (out, (gchar *)p, len); diff --git a/glib/tests/uri.c b/glib/tests/uri.c index 31bef2733..b3843b978 100644 --- a/glib/tests/uri.c +++ b/glib/tests/uri.c @@ -466,6 +466,10 @@ test_uri_escape_string (void) { ":+ \\?#", NULL, FALSE, "%3A%2B%20%5C%3F%23" }, { "a+b:c", "+", FALSE, "a+b%3Ac" }, { "a+b:c\303\234", "+", TRUE, "a+b%3Ac\303\234" }, + /* Incomplete UTF-8 sequence: */ + { "\xfc\x3b\xd2", NULL, TRUE, "%FC%3B%D2" }, + /* Invalid sequence: */ + { "\xc3\xb1\xc3\x28", NULL, TRUE, "ñ%C3%28" }, }; gsize i; From 30a3a1360e7efbdf1897c0193f2b42deb5cfc2e1 Mon Sep 17 00:00:00 2001 From: Philip Withnall Date: Mon, 5 Oct 2020 13:52:38 +0100 Subject: [PATCH 3/3] guri: Add additional tests for scope ID parsing These bump up the code coverage. Signed-off-by: Philip Withnall --- glib/tests/uri.c | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/glib/tests/uri.c b/glib/tests/uri.c index b3843b978..b8a0c6a47 100644 --- a/glib/tests/uri.c +++ b/glib/tests/uri.c @@ -749,6 +749,14 @@ static const UriAbsoluteTest absolute_tests[] = { { NULL, NULL, NULL, -1, NULL, NULL, NULL } }, { "http://[192.168.0.1%25em1]/", G_URI_FLAGS_NONE, FALSE, G_URI_ERROR_BAD_HOST, { NULL, NULL, NULL, -1, NULL, NULL, NULL } }, + { "http://[fe80::dead:beef%2em1]/", G_URI_FLAGS_PARSE_RELAXED, TRUE, 0, + { "http", NULL, "fe80::dead:beef%2em1", -1, "/", NULL, NULL } }, + { "http://[fe80::dead:beef%2em1]/", G_URI_FLAGS_NONE, FALSE, G_URI_ERROR_BAD_HOST, + { NULL, NULL, NULL, -1, NULL, NULL, NULL } }, + { "http://[fe80::dead:beef%25em1%00]/", G_URI_FLAGS_PARSE_RELAXED, FALSE, G_URI_ERROR_BAD_HOST, + { NULL, NULL, NULL, -1, NULL, NULL, NULL } }, + { "http://[fe80::dead:beef%25em1%00]/", G_URI_FLAGS_NONE, FALSE, G_URI_ERROR_BAD_HOST, + { NULL, NULL, NULL, -1, NULL, NULL, NULL } }, }; static void