From 7f6f525f2134d4be84911987a0b5b17ace46f348 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Fri, 9 Apr 2021 17:09:57 +0100 Subject: [PATCH] Add fu_bytes_get_data_safe() to check for zero sized data Fixes https://github.com/fwupd/fwupd/issues/3122 --- libfwupdplugin/fu-common.c | 32 ++++++++++++++++++++++++++ libfwupdplugin/fu-common.h | 3 +++ libfwupdplugin/fu-self-test.c | 40 +++++++++++++++++++++++++++++++++ libfwupdplugin/fwupdplugin.map | 1 + plugins/uefi-dbx/fu-efi-image.c | 4 +++- 5 files changed, 79 insertions(+), 1 deletion(-) diff --git a/libfwupdplugin/fu-common.c b/libfwupdplugin/fu-common.c index b4e4064ff..2bcc96569 100644 --- a/libfwupdplugin/fu-common.c +++ b/libfwupdplugin/fu-common.c @@ -3306,6 +3306,38 @@ fu_battery_state_to_string (FuBatteryState battery_state) return NULL; } +/** + * fu_bytes_get_data_safe: + * @bytes: a #GBytes + * @bufsz: (out) (optional): location to return size of byte data + * @error: A #GError or %NULL + * + * Get the byte data in the #GBytes. This data should not be modified. + * This function will always return the same pointer for a given #GBytes. + * + * If the size of @bytes is zero, then %NULL is returned and the @error is set, + * which differs in behaviour to that of g_bytes_get_data(). + * + * This may be useful when calling g_mapped_file_new() on a zero-length file. + * + * Returns: a pointer to the byte data, or %NULL. + * + * Since: 1.6.0 + **/ +const guint8 * +fu_bytes_get_data_safe (GBytes *bytes, gsize *bufsz, GError **error) +{ + const guint8 *buf = g_bytes_get_data (bytes, bufsz); + if (buf == NULL) { + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_INVALID_DATA, + "invalid data"); + return NULL; + } + return buf; +} + /** * fu_xmlb_builder_insert_kv: * @bn: #JsonBuilder diff --git a/libfwupdplugin/fu-common.h b/libfwupdplugin/fu-common.h index 1dc9f471b..f84ae5164 100644 --- a/libfwupdplugin/fu-common.h +++ b/libfwupdplugin/fu-common.h @@ -201,6 +201,9 @@ void fu_common_dump_bytes (const gchar *log_domain, GBytes *fu_common_bytes_align (GBytes *bytes, gsize blksz, gchar padval); +const guint8 *fu_bytes_get_data_safe (GBytes *bytes, + gsize *bufsz, + GError **error); gboolean fu_common_bytes_is_empty (GBytes *bytes); gboolean fu_common_bytes_compare (GBytes *bytes1, GBytes *bytes2, diff --git a/libfwupdplugin/fu-self-test.c b/libfwupdplugin/fu-self-test.c index 0bc6b660c..c8670aad8 100644 --- a/libfwupdplugin/fu-self-test.c +++ b/libfwupdplugin/fu-self-test.c @@ -1068,6 +1068,45 @@ fu_common_store_cab_error_missing_file_func (void) g_assert_null (silo); } +static void +fu_common_bytes_get_data_func (void) +{ + const gchar *fn = "/tmp/fwupdzero"; + const guint8 *buf; + gboolean ret; + g_autoptr(GBytes) bytes1 = NULL; + g_autoptr(GBytes) bytes2 = NULL; + g_autoptr(GBytes) bytes3 = NULL; + g_autoptr(GError) error = NULL; + g_autoptr(GMappedFile) mmap = NULL; + + /* create file with zero size */ + ret = g_file_set_contents (fn, NULL, 0, &error); + g_assert_no_error (error); + g_assert_true (ret); + + /* check we got zero sized data */ + bytes1 = fu_common_get_contents_bytes (fn, &error); + g_assert_no_error (error); + g_assert_nonnull (bytes1); + g_assert_cmpint (g_bytes_get_size (bytes1), ==, 0); + g_assert_nonnull (g_bytes_get_data (bytes1, NULL)); + + /* do the same with an mmap mapping, which returns NULL on empty file */ + mmap = g_mapped_file_new (fn, FALSE, &error); + g_assert_no_error (error); + g_assert_nonnull (mmap); + bytes2 = g_mapped_file_get_bytes (mmap); + g_assert_nonnull (bytes2); + g_assert_cmpint (g_bytes_get_size (bytes2), ==, 0); + g_assert_null (g_bytes_get_data (bytes2, NULL)); + + /* use the safe function */ + buf = fu_bytes_get_data_safe (bytes2, NULL, &error); + g_assert_error (error, G_IO_ERROR, G_IO_ERROR_INVALID_DATA); + g_assert_null (buf); +} + static void fu_common_store_cab_error_size_func (void) { @@ -2664,6 +2703,7 @@ main (int argc, char **argv) g_test_add_func ("/fwupd/common{cab-error-wrong-checksum}", fu_common_store_cab_error_wrong_checksum_func); g_test_add_func ("/fwupd/common{cab-error-missing-file}", fu_common_store_cab_error_missing_file_func); g_test_add_func ("/fwupd/common{cab-error-size}", fu_common_store_cab_error_size_func); + g_test_add_func ("/fwupd/common{bytes-get-data}", fu_common_bytes_get_data_func); g_test_add_func ("/fwupd/common{spawn)", fu_common_spawn_func); g_test_add_func ("/fwupd/common{spawn-timeout)", fu_common_spawn_timeout_func); g_test_add_func ("/fwupd/common{firmware-builder}", fu_common_firmware_builder_func); diff --git a/libfwupdplugin/fwupdplugin.map b/libfwupdplugin/fwupdplugin.map index 385ad4148..915c1940e 100644 --- a/libfwupdplugin/fwupdplugin.map +++ b/libfwupdplugin/fwupdplugin.map @@ -716,6 +716,7 @@ LIBFWUPDPLUGIN_1.6.0 { fu_battery_state_to_string; fu_byte_array_align_up; fu_byte_array_set_size_full; + fu_bytes_get_data_safe; fu_common_align_up; fu_context_add_compile_version; fu_context_add_firmware_gtype; diff --git a/plugins/uefi-dbx/fu-efi-image.c b/plugins/uefi-dbx/fu-efi-image.c index 31229316b..8297099f4 100644 --- a/plugins/uefi-dbx/fu-efi-image.c +++ b/plugins/uefi-dbx/fu-efi-image.c @@ -110,7 +110,9 @@ fu_efi_image_new (GBytes *data, GError **error) g_autoptr(GPtrArray) checksum_regions = NULL; /* verify this is a DOS file */ - buf = g_bytes_get_data (data, &bufsz); + buf = fu_bytes_get_data_safe (data, &bufsz, error); + if (buf == NULL) + return NULL; if (!fu_common_read_uint16_safe (buf, bufsz, _DOS_OFFSET_SIGNATURE, &dos_sig, G_LITTLE_ENDIAN, error))