From 119d260bd2d3781e9b24d1cc3dc1cbf11787e774 Mon Sep 17 00:00:00 2001 From: Richard Hughes Date: Wed, 17 Mar 2021 10:07:38 +0000 Subject: [PATCH] trivial: Limit alignment to 2GB to fix a fuzzing crash This meant defining alignment values in FuFirmware. Fixes https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=32131 --- libfwupdplugin/fu-common.c | 5 +++- libfwupdplugin/fu-firmware.c | 12 +++++++- libfwupdplugin/fu-firmware.h | 34 +++++++++++++++++++++++ plugins/bcm57xx/fu-bcm57xx-stage1-image.c | 2 +- plugins/spi/fu-efi-firmware-file.c | 2 +- plugins/spi/fu-efi-firmware-filesystem.c | 2 +- plugins/spi/fu-efi-firmware-section.c | 2 +- plugins/spi/fu-efi-firmware-volume.c | 13 ++++++++- plugins/spi/fu-ifd-bios.c | 2 +- plugins/spi/fu-ifd-image.c | 2 +- 10 files changed, 67 insertions(+), 9 deletions(-) diff --git a/libfwupdplugin/fu-common.c b/libfwupdplugin/fu-common.c index 511605b02..c08524f2c 100644 --- a/libfwupdplugin/fu-common.c +++ b/libfwupdplugin/fu-common.c @@ -40,6 +40,7 @@ #include "fwupd-error.h" #include "fu-common.h" +#include "fu-firmware.h" #include "fu-volume-private.h" #define UDISKS_DBUS_SERVICE "org.freedesktop.UDisks2" @@ -3245,7 +3246,7 @@ fu_common_uri_get_scheme (const gchar *uri) /** * fu_common_align_up: * @value: value to align - * @alignment: align to this power of 2 + * @alignment: align to this power of 2, where 0x1F is the maximum value of 2GB * * Align a value to a power of 2 boundary, where @alignment is the bit position * to align to. If @alignment is zero then @value is always returned unchanged. @@ -3261,6 +3262,8 @@ fu_common_align_up (gsize value, guint8 alignment) gsize value_new; guint32 mask = 1 << alignment; + g_return_val_if_fail (alignment <= FU_FIRMWARE_ALIGNMENT_2G, G_MAXSIZE); + /* no alignment required */ if ((value & (mask - 1)) == 0) return value; diff --git a/libfwupdplugin/fu-firmware.c b/libfwupdplugin/fu-firmware.c index 7722c4d22..9447b204d 100644 --- a/libfwupdplugin/fu-firmware.c +++ b/libfwupdplugin/fu-firmware.c @@ -816,8 +816,18 @@ fu_firmware_build (FuFirmware *self, XbNode *n, GError **error) if (tmpval != G_MAXUINT64) fu_firmware_set_offset (self, tmpval); tmpval = xb_node_query_text_as_uint (n, "alignment", NULL); - if (tmpval != G_MAXUINT64) + if (tmpval != G_MAXUINT64) { + if (tmpval > FU_FIRMWARE_ALIGNMENT_2G) { + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_NOT_FOUND, + "0x%x invalid, maximum is 0x%x", + (guint) tmpval, + (guint) FU_FIRMWARE_ALIGNMENT_2G); + return FALSE; + } fu_firmware_set_alignment (self, (guint8) tmpval); + } tmp = xb_node_query_text (n, "filename", NULL); if (tmp != NULL) { g_autoptr(GBytes) blob = NULL; diff --git a/libfwupdplugin/fu-firmware.h b/libfwupdplugin/fu-firmware.h index 1e5767fb4..c41ef9116 100644 --- a/libfwupdplugin/fu-firmware.h +++ b/libfwupdplugin/fu-firmware.h @@ -83,6 +83,40 @@ typedef guint64 FuFirmwareFlags; #define FU_FIRMWARE_ID_SIGNATURE "signature" #define FU_FIRMWARE_ID_HEADER "header" +#define FU_FIRMWARE_ALIGNMENT_1 0x00 +#define FU_FIRMWARE_ALIGNMENT_2 0x01 +#define FU_FIRMWARE_ALIGNMENT_4 0x02 +#define FU_FIRMWARE_ALIGNMENT_8 0x03 +#define FU_FIRMWARE_ALIGNMENT_16 0x04 +#define FU_FIRMWARE_ALIGNMENT_32 0x05 +#define FU_FIRMWARE_ALIGNMENT_64 0x06 +#define FU_FIRMWARE_ALIGNMENT_128 0x07 +#define FU_FIRMWARE_ALIGNMENT_256 0x08 +#define FU_FIRMWARE_ALIGNMENT_512 0x09 +#define FU_FIRMWARE_ALIGNMENT_1K 0x0A +#define FU_FIRMWARE_ALIGNMENT_2K 0x0B +#define FU_FIRMWARE_ALIGNMENT_4K 0x0C +#define FU_FIRMWARE_ALIGNMENT_8K 0x0D +#define FU_FIRMWARE_ALIGNMENT_16K 0x0E +#define FU_FIRMWARE_ALIGNMENT_32K 0x0F +#define FU_FIRMWARE_ALIGNMENT_64K 0x10 +#define FU_FIRMWARE_ALIGNMENT_128K 0x11 +#define FU_FIRMWARE_ALIGNMENT_256K 0x12 +#define FU_FIRMWARE_ALIGNMENT_512K 0x13 +#define FU_FIRMWARE_ALIGNMENT_1M 0x14 +#define FU_FIRMWARE_ALIGNMENT_2M 0x15 +#define FU_FIRMWARE_ALIGNMENT_4M 0x16 +#define FU_FIRMWARE_ALIGNMENT_8M 0x17 +#define FU_FIRMWARE_ALIGNMENT_16M 0x18 +#define FU_FIRMWARE_ALIGNMENT_32M 0x19 +#define FU_FIRMWARE_ALIGNMENT_64M 0x1A +#define FU_FIRMWARE_ALIGNMENT_128M 0x1B +#define FU_FIRMWARE_ALIGNMENT_256M 0x1C +#define FU_FIRMWARE_ALIGNMENT_512M 0x1D +#define FU_FIRMWARE_ALIGNMENT_1G 0x1E +#define FU_FIRMWARE_ALIGNMENT_2G 0x1F +#define FU_FIRMWARE_ALIGNMENT_4G 0x20 + const gchar *fu_firmware_flag_to_string (FuFirmwareFlags flag); FuFirmwareFlags fu_firmware_flag_from_string (const gchar *flag); diff --git a/plugins/bcm57xx/fu-bcm57xx-stage1-image.c b/plugins/bcm57xx/fu-bcm57xx-stage1-image.c index 633442327..98a37a8ad 100644 --- a/plugins/bcm57xx/fu-bcm57xx-stage1-image.c +++ b/plugins/bcm57xx/fu-bcm57xx-stage1-image.c @@ -122,7 +122,7 @@ fu_bcm57xx_stage1_image_write (FuFirmware *firmware, GError **error) static void fu_bcm57xx_stage1_image_init (FuBcm57xxStage1Image *self) { - fu_firmware_set_alignment (FU_FIRMWARE (self), 2); + fu_firmware_set_alignment (FU_FIRMWARE (self), FU_FIRMWARE_ALIGNMENT_4); } static void diff --git a/plugins/spi/fu-efi-firmware-file.c b/plugins/spi/fu-efi-firmware-file.c index 5b25eb4e5..ba80561e8 100644 --- a/plugins/spi/fu-efi-firmware-file.c +++ b/plugins/spi/fu-efi-firmware-file.c @@ -353,7 +353,7 @@ fu_efi_firmware_file_init (FuEfiFirmwareFile *self) FuEfiFirmwareFilePrivate *priv = GET_PRIVATE (self); priv->attrib = FU_EFI_FIRMWARE_FILE_ATTRIB_NONE; priv->type = FU_EFI_FIRMWARE_FILE_TYPE_RAW; - fu_firmware_set_alignment (FU_FIRMWARE (self), 3); + fu_firmware_set_alignment (FU_FIRMWARE (self), FU_FIRMWARE_ALIGNMENT_8); } static void diff --git a/plugins/spi/fu-efi-firmware-filesystem.c b/plugins/spi/fu-efi-firmware-filesystem.c index dc6d88fa5..acef0aa6c 100644 --- a/plugins/spi/fu-efi-firmware-filesystem.c +++ b/plugins/spi/fu-efi-firmware-filesystem.c @@ -93,7 +93,7 @@ fu_efi_firmware_filesystem_write (FuFirmware *firmware, GError **error) static void fu_efi_firmware_filesystem_init (FuEfiFirmwareFilesystem *self) { - fu_firmware_set_alignment (FU_FIRMWARE (self), 3); + fu_firmware_set_alignment (FU_FIRMWARE (self), FU_FIRMWARE_ALIGNMENT_8); } static void diff --git a/plugins/spi/fu-efi-firmware-section.c b/plugins/spi/fu-efi-firmware-section.c index f41ab2411..364f79eda 100644 --- a/plugins/spi/fu-efi-firmware-section.c +++ b/plugins/spi/fu-efi-firmware-section.c @@ -252,7 +252,7 @@ fu_efi_firmware_section_init (FuEfiFirmwareSection *self) { FuEfiFirmwareSectionPrivate *priv = GET_PRIVATE (self); priv->type = FU_EFI_FIRMWARE_SECTION_TYPE_RAW; -// fu_firmware_set_alignment (FU_FIRMWARE (self), 3); +// fu_firmware_set_alignment (FU_FIRMWARE (self), FU_FIRMWARE_ALIGNMENT_8); } static void diff --git a/plugins/spi/fu-efi-firmware-volume.c b/plugins/spi/fu-efi-firmware-volume.c index e1bfc3b8e..7088ffe97 100644 --- a/plugins/spi/fu-efi-firmware-volume.c +++ b/plugins/spi/fu-efi-firmware-volume.c @@ -77,6 +77,7 @@ fu_efi_firmware_volume_parse (FuFirmware *firmware, guint32 attrs = 0; guint32 sig = 0; guint64 fv_length = 0; + guint8 alignment; guint8 revision = 0; const guint8 *buf = g_bytes_get_data (fw, &bufsz); g_autofree gchar *guid_str = NULL; @@ -129,7 +130,17 @@ fu_efi_firmware_volume_parse (FuFirmware *firmware, g_prefix_error (error, "failed to read attrs: "); return FALSE; } - fu_firmware_set_alignment (firmware, (attrs & 0x00ff0000) >> 16); + alignment = (attrs & 0x00ff0000) >> 16; + if (alignment > FU_FIRMWARE_ALIGNMENT_2G) { + g_set_error (error, + G_IO_ERROR, + G_IO_ERROR_NOT_FOUND, + "0x%x invalid, maximum is 0x%x", + (guint) alignment, + (guint) FU_FIRMWARE_ALIGNMENT_2G); + return FALSE; + } + fu_firmware_set_alignment (firmware, alignment); priv->attrs = attrs & 0xffff; if (!fu_common_read_uint16_safe (buf, bufsz, offset + FU_EFI_FIRMWARE_VOLUME_OFFSET_HDR_LEN, diff --git a/plugins/spi/fu-ifd-bios.c b/plugins/spi/fu-ifd-bios.c index 470ca4f8e..ab349602f 100644 --- a/plugins/spi/fu-ifd-bios.c +++ b/plugins/spi/fu-ifd-bios.c @@ -83,7 +83,7 @@ fu_ifd_bios_parse (FuFirmware *firmware, static void fu_ifd_bios_init (FuIfdBios *self) { - fu_firmware_set_alignment (FU_FIRMWARE (self), 12); + fu_firmware_set_alignment (FU_FIRMWARE (self), FU_FIRMWARE_ALIGNMENT_4K); } static void diff --git a/plugins/spi/fu-ifd-image.c b/plugins/spi/fu-ifd-image.c index bf9b012fc..a67fb8bed 100644 --- a/plugins/spi/fu-ifd-image.c +++ b/plugins/spi/fu-ifd-image.c @@ -104,7 +104,7 @@ fu_ifd_image_write (FuFirmware *firmware, GError **error) static void fu_ifd_image_init (FuIfdImage *self) { - fu_firmware_set_alignment (FU_FIRMWARE (self), 12); + fu_firmware_set_alignment (FU_FIRMWARE (self), FU_FIRMWARE_ALIGNMENT_4K); } static void