1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101
|
From: Milan Crha <mcrha@redhat.com>
Date: Tue, 13 May 2025 10:38:49 +0200
Subject: soup-form: Fix a possible memory leak in
soup_form_decode_multipart()
The output variables can be set multiple times, when there are multiparts
with the same name, thus first clear any previously value and only then
assign a new value.
Origin: upstream, 3.7.0, commit:66b5c5be947062df9caf7025b56ee1de32aee3ac
Bug: https://gitlab.gnome.org/GNOME/libsoup/-/issues/446
---
libsoup/soup-form.c | 12 +++++++++---
tests/forms-test.c | 41 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 50 insertions(+), 3 deletions(-)
diff --git a/libsoup/soup-form.c b/libsoup/soup-form.c
index 2eb5d57..98130c8 100644
--- a/libsoup/soup-form.c
+++ b/libsoup/soup-form.c
@@ -168,12 +168,18 @@ soup_form_decode_multipart (SoupMultipart *multipart,
}
if (file_control_name && !strcmp (name, file_control_name)) {
- if (filename)
+ if (filename) {
+ g_free (*filename);
*filename = g_strdup (g_hash_table_lookup (params, "filename"));
- if (content_type)
+ }
+ if (content_type) {
+ g_free (*content_type);
*content_type = g_strdup (soup_message_headers_get_content_type (part_headers, NULL));
- if (file)
+ }
+ if (file) {
+ g_clear_pointer (file, g_bytes_unref);
*file = g_bytes_ref (part_body);
+ }
} else {
g_hash_table_insert (form_data_set,
g_strdup (name),
diff --git a/tests/forms-test.c b/tests/forms-test.c
index 1002374..183900f 100644
--- a/tests/forms-test.c
+++ b/tests/forms-test.c
@@ -485,6 +485,46 @@ md5_callback (SoupServer *server,
soup_server_message_set_status (msg, SOUP_STATUS_METHOD_NOT_ALLOWED, NULL);
}
+static void
+do_form_decode_multipart_test (void)
+{
+ SoupMultipart *multipart = soup_multipart_new ("multipart/form-data");
+ const char *file_control_name = "uploaded_file";
+ char *content_type = NULL;
+ char *filename = NULL;
+ GBytes *file = NULL;
+ GHashTable *result;
+ int part;
+
+ for (part = 0; part < 2; part++) {
+ SoupMessageHeaders *headers = soup_message_headers_new (SOUP_MESSAGE_HEADERS_MULTIPART);
+ GHashTable *params = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free);
+ GBytes *body = g_bytes_new (NULL, 0);
+
+ g_hash_table_insert (params, g_strdup ("name"), g_strdup (file_control_name));
+ g_hash_table_insert (params, g_strdup ("filename"), g_strdup (file_control_name));
+ soup_message_headers_set_content_disposition (headers, "form-data", params);
+ soup_message_headers_set_content_type (headers, "text/x-form", NULL);
+ soup_multipart_append_part (multipart, headers, body);
+
+ soup_message_headers_unref (headers);
+ g_hash_table_destroy (params);
+ g_bytes_unref (body);
+ }
+
+ /* this would leak memory of the output variables, due to two parts having the same 'file_control_name' */
+ result = soup_form_decode_multipart (multipart, file_control_name, &filename, &content_type, &file);
+ g_assert_nonnull (result);
+ g_assert_cmpstr (content_type, ==, "text/x-form");
+ g_assert_cmpstr (filename, ==, file_control_name);
+ g_assert_nonnull (file);
+
+ g_hash_table_destroy (result);
+ g_free (content_type);
+ g_free (filename);
+ g_bytes_unref (file);
+}
+
static gboolean run_tests = TRUE;
static GOptionEntry no_test_entry[] = {
@@ -525,6 +565,7 @@ main (int argc, char **argv)
g_uri_unref (uri);
g_test_add_func ("/forms/decode", do_form_decode_test);
+ g_test_add_func ("/forms/decodemultipart", do_form_decode_multipart_test);
ret = g_test_run ();
} else {
|