From 242a10fbb12dbdc12d254bd8fc8669a0ac055304 Mon Sep 17 00:00:00 2001 From: Patrick Griffis Date: Wed, 5 Feb 2025 14:39:42 -0600 Subject: [PATCH] sniffer: Fix potential overflow --- libsoup/content-sniffer/soup-content-sniffer.c | 2 +- tests/meson.build | 4 +++- tests/resources/whitespace.html | Bin 0 -> 512 bytes tests/sniffing-test.c | 5 +++++ tests/soup-tests.gresource.xml | 1 + 5 files changed, 10 insertions(+), 2 deletions(-) create mode 100644 tests/resources/whitespace.html diff --git a/libsoup/content-sniffer/soup-content-sniffer.c b/libsoup/content-sniffer/soup-content-sniffer.c index aeee2e25..da94e60c 100644 --- a/libsoup/content-sniffer/soup-content-sniffer.c +++ b/libsoup/content-sniffer/soup-content-sniffer.c @@ -669,7 +669,7 @@ sniff_feed_or_html (SoupContentSniffer *sniffer, GBytes *buffer) pos = 3; look_for_tag: - if (pos > resource_length) + if (pos >= resource_length) goto text_html; if (skip_insignificant_space (resource, &pos, resource_length)) diff --git a/tests/meson.build b/tests/meson.build index 5aee70bc..ee118a01 100644 --- a/tests/meson.build +++ b/tests/meson.build @@ -103,7 +103,9 @@ tests = [ {'name': 'session'}, {'name': 'server-auth'}, {'name': 'server'}, - {'name': 'sniffing'}, + {'name': 'sniffing', + 'depends': [test_resources], + }, {'name': 'ssl', 'dependencies': [gnutls_dep], 'depends': mock_pkcs11_module, diff --git a/tests/sniffing-test.c b/tests/sniffing-test.c index 6116719a..b5428177 100644 --- a/tests/sniffing-test.c +++ b/tests/sniffing-test.c @@ -512,6 +512,11 @@ main (int argc, char **argv) "type/text_html; charset=UTF-8/test.html => text/html; charset=UTF-8", do_sniffing_test); + /* Test hitting skip_insignificant_space() with number of bytes equaling resource_length. */ + g_test_add_data_func ("/sniffing/whitespace", + "type/text_html/whitespace.html => text/html", + do_sniffing_test); + /* Test that disabling the sniffer works correctly */ g_test_add_data_func ("/sniffing/disabled", "/text_or_binary/home.gif", -- From c415ad0b6771992e66c70edf373566c6e247089d Mon Sep 17 00:00:00 2001 From: Patrick Griffis Date: Tue, 18 Feb 2025 14:29:50 -0600 Subject: [PATCH] sniffer: Add better coverage of skip_insignificant_space() --- .../content-sniffer/soup-content-sniffer.c | 10 ++-- tests/resources/whitespace.html | Bin 512 -> 0 bytes tests/sniffing-test.c | 53 ++++++++++++++++-- tests/soup-tests.gresource.xml | 1 - 4 files changed, 53 insertions(+), 11 deletions(-) delete mode 100644 tests/resources/whitespace.html diff --git a/libsoup/content-sniffer/soup-content-sniffer.c b/libsoup/content-sniffer/soup-content-sniffer.c index da94e60c..a5e18d5d 100644 --- a/libsoup/content-sniffer/soup-content-sniffer.c +++ b/libsoup/content-sniffer/soup-content-sniffer.c @@ -638,8 +638,11 @@ sniff_text_or_binary (SoupContentSniffer *sniffer, GBytes *buffer) } static gboolean -skip_insignificant_space (const char *resource, int *pos, int resource_length) +skip_insignificant_space (const char *resource, gsize *pos, gsize resource_length) { + if (*pos >= resource_length) + return TRUE; + while ((resource[*pos] == '\x09') || (resource[*pos] == '\x20') || (resource[*pos] == '\x0A') || @@ -659,7 +662,7 @@ sniff_feed_or_html (SoupContentSniffer *sniffer, GBytes *buffer) gsize resource_length; const char *resource = g_bytes_get_data (buffer, &resource_length); resource_length = MIN (512, resource_length); - int pos = 0; + gsize pos = 0; if (resource_length < 3) goto text_html; @@ -669,9 +672,6 @@ sniff_feed_or_html (SoupContentSniffer *sniffer, GBytes *buffer) pos = 3; look_for_tag: - if (pos >= resource_length) - goto text_html; - if (skip_insignificant_space (resource, &pos, resource_length)) goto text_html; diff --git a/tests/sniffing-test.c b/tests/sniffing-test.c index b5428177..7857732d 100644 --- a/tests/sniffing-test.c +++ b/tests/sniffing-test.c @@ -342,6 +342,52 @@ test_disabled (gconstpointer data) g_uri_unref (uri); } +static const gsize MARKUP_LENGTH = strlen (""); + +static void +do_skip_whitespace_test (void) +{ + SoupContentSniffer *sniffer = soup_content_sniffer_new (); + SoupMessage *msg = soup_message_new (SOUP_METHOD_GET, "http://example.org"); + const char *test_cases[] = { + "", + "$trailing_data + memcpy (p, "", strlen ("-->")); + p += strlen ("-->"); + if (strlen (trailing_data)) + memcpy (p, trailing_data, strlen (trailing_data)); + // Purposefully not NUL terminated. + + buffer = g_bytes_new_take (g_steal_pointer (&data), testsize); + content_type = soup_content_sniffer_sniff (sniffer, msg, buffer, NULL); + + g_free (content_type); + g_bytes_unref (buffer); + } + + g_object_unref (msg); + g_object_unref (sniffer); +} + int main (int argc, char **argv) { @@ -512,16 +558,13 @@ main (int argc, char **argv) "type/text_html; charset=UTF-8/test.html => text/html; charset=UTF-8", do_sniffing_test); - /* Test hitting skip_insignificant_space() with number of bytes equaling resource_length. */ - g_test_add_data_func ("/sniffing/whitespace", - "type/text_html/whitespace.html => text/html", - do_sniffing_test); - /* Test that disabling the sniffer works correctly */ g_test_add_data_func ("/sniffing/disabled", "/text_or_binary/home.gif", test_disabled); + g_test_add_func ("/sniffing/whitespace", do_skip_whitespace_test); + ret = g_test_run (); g_uri_unref (base_uri); -- 2.49.0