[libsoup/pgriffis/utf8-safe-apis: 5/5] headers: Ensure untrusted header values are UTF-8

[Patrick Griffis <pgriffis src gnome org>]

commit fe8491d86f3c99997126818f9af614964f0a50a0
Author: Patrick Griffis <pgriffis igalia com>
Date:   Thu Jul 22 13:51:00 2021 -0500

    headers: Ensure untrusted header values are UTF-8
    
    Our API uses `char *` for all headers throughout.
    This means that GObject-Introspection assumes it it is valid UTF-8,
    so languages assume it is valid UTF-8.
    Applications using the C API assume it is valid UTF-8.
    
    Passing along unverified bytes is unsafe and will cause issues.

 libsoup/http2/soup-client-message-io-http2.c |  5 ++--
 libsoup/soup-headers.c                       |  3 ++-
 libsoup/soup-message-headers-private.h       |  3 +++
 libsoup/soup-message-headers.c               | 15 +++++++++++
 tests/misc-test.c                            | 39 ++++++++++++++++++++++++++++
 5 files changed, 62 insertions(+), 3 deletions(-)
---
diff --git a/libsoup/http2/soup-client-message-io-http2.c b/libsoup/http2/soup-client-message-io-http2.c
index 592fde68..5e7f0665 100644
--- a/libsoup/http2/soup-client-message-io-http2.c
+++ b/libsoup/http2/soup-client-message-io-http2.c
@@ -32,6 +32,7 @@
 
 #include "soup-body-input-stream.h"
 #include "soup-message-metrics-private.h"
+#include "soup-message-headers-private.h"
 #include "soup-message-private.h"
 #include "soup-message-io-source.h"
 #include "soup-message-queue-item.h"
@@ -544,8 +545,8 @@ on_header_callback (nghttp2_session     *session,
                 return 0;
         }
 
-        soup_message_headers_append (soup_message_get_response_headers (data->msg),
-                                     (const char *)name, (const char *)value);
+        soup_message_headers_append_untrusted_data (soup_message_get_response_headers (data->msg),
+                                                    (const char*)name, (const char*)value);
         return 0;
 }
 
diff --git a/libsoup/soup-headers.c b/libsoup/soup-headers.c
index e7f843b0..87779fcf 100644
--- a/libsoup/soup-headers.c
+++ b/libsoup/soup-headers.c
@@ -14,6 +14,7 @@
 
 #include "soup-misc.h"
 #include "soup-headers.h"
+#include "soup-message-headers-private.h"
 #include "soup.h"
 
 /**
@@ -154,7 +155,7 @@ soup_headers_parse (const char *str, int len, SoupMessageHeaders *dest)
                for (p = strchr (value, '\r'); p; p = strchr (p, '\r'))
                        *p = ' ';
 
-               soup_message_headers_append (dest, name, value);
+               soup_message_headers_append_untrusted_data (dest, name, value);
         }
        success = TRUE;
 
diff --git a/libsoup/soup-message-headers-private.h b/libsoup/soup-message-headers-private.h
index b606a331..98154645 100644
--- a/libsoup/soup-message-headers-private.h
+++ b/libsoup/soup-message-headers-private.h
@@ -10,6 +10,9 @@
 
 G_BEGIN_DECLS
 
+void        soup_message_headers_append_untrusted_data  (SoupMessageHeaders *hdrs,
+                                                         const char         *name,
+                                                         const char         *value);
 void        soup_message_headers_append_common          (SoupMessageHeaders *hdrs,
                                                          SoupHeaderName      name,
                                                          const char         *value);
diff --git a/libsoup/soup-message-headers.c b/libsoup/soup-message-headers.c
index ebcecb9f..3ebfc7de 100644
--- a/libsoup/soup-message-headers.c
+++ b/libsoup/soup-message-headers.c
@@ -353,6 +353,21 @@ soup_message_headers_append (SoupMessageHeaders *hdrs,
                g_hash_table_remove (hdrs->uncommon_concat, header.name);
 }
 
+/*
+ * Appends a header value ensuring that it is valid UTF8.
+ */
+void
+soup_message_headers_append_untrusted_data (SoupMessageHeaders *hdrs,
+                                            const char         *name,
+                                            const char         *value)
+{
+        char *safe_value = g_utf8_make_valid (value, -1);
+        char *safe_name = g_utf8_make_valid (name, -1);
+        soup_message_headers_append (hdrs, safe_name, safe_value);
+        g_free (safe_value);
+        g_free (safe_name);
+}
+
 void
 soup_message_headers_replace_common (SoupMessageHeaders *hdrs,
                                      SoupHeaderName      name,
diff --git a/tests/misc-test.c b/tests/misc-test.c
index 593b4f8f..3ab8fcab 100644
--- a/tests/misc-test.c
+++ b/tests/misc-test.c
@@ -6,6 +6,7 @@
 #include "test-utils.h"
 #include "soup-connection.h"
 #include "soup-session-private.h"
+#include "soup-message-headers-private.h"
 
 SoupServer *server;
 GUri *base_uri;
@@ -55,6 +56,15 @@ server_callback (SoupServer        *server,
                 g_source_unref (timeout);
        }
 
+        if (!strcmp (path, "/invalid_utf8_headers")) {
+                SoupMessageHeaders *headers = soup_server_message_get_response_headers (msg);
+                const char *invalid_utf8_value = "\xe2\x82\xa0gh\xe2\xffjl";
+
+                /* Purposefully insert invalid utf8 data */
+                g_assert_false (g_utf8_validate (invalid_utf8_value, -1, NULL));
+                soup_message_headers_append (headers, "InvalidValue", invalid_utf8_value);
+        }
+
        soup_server_message_set_status (msg, SOUP_STATUS_OK, NULL);
        if (!strcmp (g_uri_get_host (uri), "foo")) {
                soup_server_message_set_response (msg, "text/plain",
@@ -933,6 +943,34 @@ do_response_informational_content_length_test (void)
         soup_test_server_quit_unref (server);
 }
 
+static void
+do_invalid_utf8_headers_test (void)
+{
+        SoupSession *session;
+        SoupMessage *msg;
+        GUri *uri;
+        SoupMessageHeaders *headers;
+        guint status;
+        const char *header_value;
+
+        session = soup_test_session_new (NULL);
+
+        uri = g_uri_parse_relative (base_uri, "/invalid_utf8_headers", SOUP_HTTP_URI_FLAGS, NULL);
+        msg = soup_message_new_from_uri ("GET", uri);
+
+        status = soup_test_session_send_message (session, msg);
+        g_assert_cmpuint (status, ==, SOUP_STATUS_OK);
+
+        headers = soup_message_get_response_headers (msg);
+        header_value = soup_message_headers_get_one (headers, "InvalidValue");
+        g_assert_nonnull (header_value);
+        g_assert_true (g_utf8_validate (header_value, -1, NULL));
+
+        g_object_unref (msg);
+        g_uri_unref (uri);
+        soup_test_session_abort_unref (session);
+}
+
 int
 main (int argc, char **argv)
 {
@@ -969,6 +1007,7 @@ main (int argc, char **argv)
         g_test_add_func ("/misc/remote-address", do_remote_address_test);
         g_test_add_func ("/misc/new-request-on-redirect", do_new_request_on_redirect_test);
         g_test_add_func ("/misc/response/informational/content-length", 
do_response_informational_content_length_test);
+        g_test_add_func ("/misc/invalid-utf8-headers", do_invalid_utf8_headers_test);
 
        ret = g_test_run ();