trivial: Add fu_common_bytes_new_offset()

This is a safer version of g_bytes_new_from_bytes() which returns a GError
if the offsets are invalid rather than emitting a critical warning.

This prevents a critical warning and potential crash when parsing invalid
bcm57xx firmware.
This commit is contained in:
Richard Hughes 2020-12-08 18:37:02 +00:00
parent faf8a460f9
commit 05e3377d11
16 changed files with 161 additions and 32 deletions

View File

@ -1632,6 +1632,42 @@ fu_common_bytes_pad (GBytes *bytes, gsize sz)
return g_bytes_ref (bytes); return g_bytes_ref (bytes);
} }
/**
* fu_common_bytes_new_offset:
* @bytes: a #GBytes
* @offset: where subsection starts at
* @length: length of subsection
* @error: A #GError or %NULL
*
* Creates a #GBytes which is a subsection of another #GBytes.
*
* Return value: (transfer full): a #GBytes, or #NULL if range is invalid
*
* Since: 1.5.4
**/
GBytes *
fu_common_bytes_new_offset (GBytes *bytes,
gsize offset,
gsize length,
GError **error)
{
g_return_val_if_fail (bytes != NULL, NULL);
/* sanity check */
if (offset + length > g_bytes_get_size (bytes)) {
g_set_error (error,
G_IO_ERROR,
G_IO_ERROR_INVALID_DATA,
"cannot create bytes @0x%02x for 0x%02x "
"as buffer only 0x%04x bytes in size",
(guint) offset,
(guint) length,
(guint) g_bytes_get_size (bytes));
return NULL;
}
return g_bytes_new_from_bytes (bytes, offset, length);
}
/** /**
* fu_common_realpath: * fu_common_realpath:
* @filename: a filename * @filename: a filename

View File

@ -155,6 +155,10 @@ gboolean fu_common_bytes_compare_raw (const guint8 *buf1,
GError **error); GError **error);
GBytes *fu_common_bytes_pad (GBytes *bytes, GBytes *fu_common_bytes_pad (GBytes *bytes,
gsize sz); gsize sz);
GBytes *fu_common_bytes_new_offset (GBytes *bytes,
gsize offset,
gsize length,
GError **error);
gsize fu_common_strwidth (const gchar *text); gsize fu_common_strwidth (const gchar *text);
gboolean fu_memcpy_safe (guint8 *dst, gboolean fu_memcpy_safe (guint8 *dst,
gsize dst_sz, gsize dst_sz,

View File

@ -264,7 +264,9 @@ fu_dfu_firmware_parse (FuFirmware *firmware,
} }
/* success */ /* success */
contents = g_bytes_new_from_bytes (fw, 0, len - ftr.len); contents = fu_common_bytes_new_offset (fw, 0, len - ftr.len, error);
if (contents == NULL)
return FALSE;
image = fu_firmware_image_new (contents); image = fu_firmware_image_new (contents);
fu_firmware_add_image (firmware, image); fu_firmware_add_image (firmware, image);
return TRUE; return TRUE;

View File

@ -479,11 +479,18 @@ fu_firmware_image_write_chunk (FuFirmwareImage *self,
/* if we have less data than requested */ /* if we have less data than requested */
chunk_left = g_bytes_get_size (priv->bytes) - offset; chunk_left = g_bytes_get_size (priv->bytes) - offset;
if (chunk_sz_max > chunk_left) if (chunk_sz_max > chunk_left) {
return g_bytes_new_from_bytes (priv->bytes, offset, chunk_left); return fu_common_bytes_new_offset (priv->bytes,
offset,
chunk_left,
error);
}
/* check chunk */ /* check chunk */
return g_bytes_new_from_bytes (priv->bytes, offset, chunk_sz_max); return fu_common_bytes_new_offset (priv->bytes,
offset,
chunk_sz_max,
error);
} }
void void

View File

@ -6,6 +6,7 @@
#include "config.h" #include "config.h"
#include "fu-common.h"
#include "fu-fmap-firmware.h" #include "fu-fmap-firmware.h"
#define FMAP_SIGNATURE "__FMAP__" #define FMAP_SIGNATURE "__FMAP__"
@ -217,9 +218,12 @@ fu_fmap_firmware_parse (FuFirmware *firmware,
g_autoptr(GBytes) bytes = NULL; g_autoptr(GBytes) bytes = NULL;
img = fu_firmware_image_new (NULL); img = fu_firmware_image_new (NULL);
bytes = g_bytes_new_from_bytes (fw, bytes = fu_common_bytes_new_offset (fw,
(gsize) area->offset, (gsize) area->offset,
(gsize) area->size); (gsize) area->size,
error);
if (bytes == NULL)
return FALSE;
fu_firmware_image_set_id (img, (const gchar *) area->name); fu_firmware_image_set_id (img, (const gchar *) area->name);
fu_firmware_image_set_idx (img, i + 1); fu_firmware_image_set_idx (img, i + 1);
fu_firmware_image_set_addr (img, (guint64) area->offset); fu_firmware_image_set_addr (img, (guint64) area->offset);

View File

@ -693,3 +693,9 @@ LIBFWUPDPLUGIN_1.5.3 {
fu_udev_device_get_driver; fu_udev_device_get_driver;
local: *; local: *;
} LIBFWUPDPLUGIN_1.5.2; } LIBFWUPDPLUGIN_1.5.2;
LIBFWUPDPLUGIN_1.5.4 {
global:
fu_common_bytes_new_offset;
local: *;
} LIBFWUPDPLUGIN_1.5.3;

View File

@ -40,7 +40,11 @@ fu_bcm57xx_dict_image_parse (FuFirmwareImage *image,
if (!fu_bcm57xx_verify_crc (fw, error)) if (!fu_bcm57xx_verify_crc (fw, error))
return FALSE; return FALSE;
} }
fw_nocrc = g_bytes_new_from_bytes (fw, 0x0, g_bytes_get_size (fw) - sizeof(guint32)); fw_nocrc = fu_common_bytes_new_offset (fw, 0x0,
g_bytes_get_size (fw) - sizeof(guint32),
error);
if (fw_nocrc == NULL)
return FALSE;
fu_firmware_image_set_bytes (image, fw_nocrc); fu_firmware_image_set_bytes (image, fw_nocrc);
return TRUE; return TRUE;
} }

View File

@ -130,7 +130,9 @@ fu_bcm57xx_firmware_parse_stage1 (FuBcm57xxFirmware *self,
} }
/* verify CRC */ /* verify CRC */
blob = g_bytes_new_from_bytes (fw, stage1_off, stage1_sz); blob = fu_common_bytes_new_offset (fw, stage1_off, stage1_sz, error);
if (blob == NULL)
return NULL;
if (!fu_firmware_image_parse (img, blob, flags, error)) if (!fu_firmware_image_parse (img, blob, flags, error))
return NULL; return NULL;
@ -174,7 +176,9 @@ fu_bcm57xx_firmware_parse_stage2 (FuBcm57xxFirmware *self,
} }
/* verify CRC */ /* verify CRC */
blob = g_bytes_new_from_bytes (fw, stage2_off + 0x8, stage2_sz); blob = fu_common_bytes_new_offset (fw, stage2_off + 0x8, stage2_sz, error);
if (blob == NULL)
return NULL;
if (!fu_firmware_image_parse (img, blob, flags, error)) if (!fu_firmware_image_parse (img, blob, flags, error))
return NULL; return NULL;
@ -240,7 +244,9 @@ fu_bcm57xx_firmware_parse_dict (FuBcm57xxFirmware *self, GBytes *fw, guint idx,
(guint) dict_sz, (guint) dict_off); (guint) dict_sz, (guint) dict_off);
return FALSE; return FALSE;
} }
blob = g_bytes_new_from_bytes (fw, dict_off, dict_sz); blob = fu_common_bytes_new_offset (fw, dict_off, dict_sz, error);
if (blob == NULL)
return FALSE;
if (!fu_firmware_image_parse (img, blob, flags, error)) if (!fu_firmware_image_parse (img, blob, flags, error))
return FALSE; return FALSE;
@ -312,14 +318,24 @@ fu_bcm57xx_firmware_parse (FuFirmware *firmware,
self->source_padchar = buf[bufsz - 1]; self->source_padchar = buf[bufsz - 1];
/* NVRAM header */ /* NVRAM header */
blob_header = g_bytes_new_from_bytes (fw, BCM_NVRAM_HEADER_BASE, BCM_NVRAM_HEADER_SZ); blob_header = fu_common_bytes_new_offset (fw,
BCM_NVRAM_HEADER_BASE,
BCM_NVRAM_HEADER_SZ,
error);
if (blob_header == NULL)
return FALSE;
if (!fu_bcm57xx_firmware_parse_header (self, blob_header, error)) { if (!fu_bcm57xx_firmware_parse_header (self, blob_header, error)) {
g_prefix_error (error, "failed to parse header: "); g_prefix_error (error, "failed to parse header: ");
return FALSE; return FALSE;
} }
/* info */ /* info */
blob_info = g_bytes_new_from_bytes (fw, BCM_NVRAM_INFO_BASE, BCM_NVRAM_INFO_SZ); blob_info = fu_common_bytes_new_offset (fw,
BCM_NVRAM_INFO_BASE,
BCM_NVRAM_INFO_SZ,
error);
if (blob_info == NULL)
return FALSE;
img_info = fu_bcm57xx_firmware_parse_info (self, blob_info, error); img_info = fu_bcm57xx_firmware_parse_info (self, blob_info, error);
if (img_info == NULL) { if (img_info == NULL) {
g_prefix_error (error, "failed to parse info: "); g_prefix_error (error, "failed to parse info: ");
@ -329,14 +345,24 @@ fu_bcm57xx_firmware_parse (FuFirmware *firmware,
fu_firmware_add_image (firmware, img_info); fu_firmware_add_image (firmware, img_info);
/* VPD */ /* VPD */
blob_vpd = g_bytes_new_from_bytes (fw, BCM_NVRAM_VPD_BASE, BCM_NVRAM_VPD_SZ); blob_vpd = fu_common_bytes_new_offset (fw,
BCM_NVRAM_VPD_BASE,
BCM_NVRAM_VPD_SZ,
error);
if (blob_vpd == NULL)
return FALSE;
img_vpd = fu_firmware_image_new (blob_vpd); img_vpd = fu_firmware_image_new (blob_vpd);
fu_firmware_image_set_id (img_vpd, "vpd"); fu_firmware_image_set_id (img_vpd, "vpd");
fu_firmware_image_set_offset (img_vpd, BCM_NVRAM_VPD_BASE); fu_firmware_image_set_offset (img_vpd, BCM_NVRAM_VPD_BASE);
fu_firmware_add_image (firmware, img_vpd); fu_firmware_add_image (firmware, img_vpd);
/* info2 */ /* info2 */
blob_info2 = g_bytes_new_from_bytes (fw, BCM_NVRAM_INFO2_BASE, BCM_NVRAM_INFO2_SZ); blob_info2 = fu_common_bytes_new_offset (fw,
BCM_NVRAM_INFO2_BASE,
BCM_NVRAM_INFO2_SZ,
error);
if (blob_info2 == NULL)
return FALSE;
img_info2 = fu_firmware_image_new (blob_info2); img_info2 = fu_firmware_image_new (blob_info2);
fu_firmware_image_set_id (img_info2, "info2"); fu_firmware_image_set_id (img_info2, "info2");
fu_firmware_image_set_offset (img_info2, BCM_NVRAM_INFO2_BASE); fu_firmware_image_set_offset (img_info2, BCM_NVRAM_INFO2_BASE);

View File

@ -69,7 +69,11 @@ fu_bcm57xx_stage1_image_parse (FuFirmwareImage *image,
fu_firmware_image_set_version (image, veritem->version); fu_firmware_image_set_version (image, veritem->version);
} }
fw_nocrc = g_bytes_new_from_bytes (fw, 0x0, g_bytes_get_size (fw) - sizeof(guint32)); fw_nocrc = fu_common_bytes_new_offset (fw, 0x0,
g_bytes_get_size (fw) - sizeof(guint32),
error);
if (fw_nocrc == NULL)
return FALSE;
fu_firmware_image_set_bytes (image, fw_nocrc); fu_firmware_image_set_bytes (image, fw_nocrc);
return TRUE; return TRUE;
} }

View File

@ -28,7 +28,11 @@ fu_bcm57xx_stage2_image_parse (FuFirmwareImage *image,
if (!fu_bcm57xx_verify_crc (fw, error)) if (!fu_bcm57xx_verify_crc (fw, error))
return FALSE; return FALSE;
} }
fw_nocrc = g_bytes_new_from_bytes (fw, 0x0, g_bytes_get_size (fw) - sizeof(guint32)); fw_nocrc = fu_common_bytes_new_offset (fw, 0x0,
g_bytes_get_size (fw) - sizeof(guint32),
error);
if (fw_nocrc == NULL)
return FALSE;
fu_firmware_image_set_bytes (image, fw_nocrc); fu_firmware_image_set_bytes (image, fw_nocrc);
return TRUE; return TRUE;
} }

View File

@ -479,9 +479,12 @@ fu_cros_ec_usb_device_transfer_block (FuDevice *device, gpointer user_data,
return FALSE; return FALSE;
} }
block_bytes = g_bytes_new_from_bytes (block_info->image_bytes, block_bytes = fu_common_bytes_new_offset (block_info->image_bytes,
block_info->offset, block_info->offset,
block_info->payload_size); block_info->payload_size,
error);
if (block_bytes == NULL)
return FALSE;
chunks = fu_chunk_array_new_from_bytes (block_bytes, chunks = fu_chunk_array_new_from_bytes (block_bytes,
0x00, 0x00,
0x00, 0x00,

View File

@ -194,10 +194,16 @@ dfu_target_stm_upload_element (DfuTarget *target,
/* create new image */ /* create new image */
contents = dfu_utils_bytes_join_array (chunks); contents = dfu_utils_bytes_join_array (chunks);
if (expected_size > 0) if (expected_size > 0) {
contents_truncated = g_bytes_new_from_bytes (contents, 0, expected_size); contents_truncated = fu_common_bytes_new_offset (contents,
else 0,
expected_size,
error);
if (contents_truncated == NULL)
return NULL;
} else {
contents_truncated = g_bytes_ref (contents); contents_truncated = g_bytes_ref (contents);
}
element = dfu_element_new (); element = dfu_element_new ();
dfu_element_set_contents (element, contents_truncated); dfu_element_set_contents (element, contents_truncated);
dfu_element_set_address (element, address); dfu_element_set_address (element, address);
@ -347,7 +353,12 @@ dfu_target_stm_download_element (DfuTarget *target,
length = g_bytes_get_size (bytes) - offset; length = g_bytes_get_size (bytes) - offset;
if (length > transfer_size) if (length > transfer_size)
length = transfer_size; length = transfer_size;
bytes_tmp = g_bytes_new_from_bytes (bytes, offset, length); bytes_tmp = fu_common_bytes_new_offset (bytes,
offset,
length,
error);
if (bytes_tmp == NULL)
return FALSE;
g_debug ("writing sector at 0x%04x (0x%" G_GSIZE_FORMAT ")", g_debug ("writing sector at 0x%04x (0x%" G_GSIZE_FORMAT ")",
offset_dev, offset_dev,
g_bytes_get_size (bytes_tmp)); g_bytes_get_size (bytes_tmp));

View File

@ -1176,7 +1176,12 @@ dfu_target_download_element_dfu (DfuTarget *target,
length = g_bytes_get_size (bytes) - offset; length = g_bytes_get_size (bytes) - offset;
if (length > transfer_size) if (length > transfer_size)
length = transfer_size; length = transfer_size;
bytes_tmp = g_bytes_new_from_bytes (bytes, offset, length); bytes_tmp = fu_common_bytes_new_offset (bytes,
offset,
length,
error);
if (bytes_tmp == NULL)
return FALSE;
} else { } else {
bytes_tmp = g_bytes_new (NULL, 0); bytes_tmp = g_bytes_new (NULL, 0);
} }

View File

@ -6,6 +6,7 @@
#include "config.h" #include "config.h"
#include "fu-common.h"
#include "fu-ebitdo-firmware.h" #include "fu-ebitdo-firmware.h"
struct _FuEbitdoFirmware { struct _FuEbitdoFirmware {
@ -77,13 +78,22 @@ fu_ebitdo_firmware_parse (FuFirmware *firmware,
fu_firmware_set_version (firmware, version); fu_firmware_set_version (firmware, version);
/* add header */ /* add header */
fw_hdr = g_bytes_new_from_bytes (fw, 0x0, sizeof(FuEbitdoFirmwareHeader)); fw_hdr = fu_common_bytes_new_offset (fw, 0x0,
sizeof(FuEbitdoFirmwareHeader),
error);
if (fw_hdr == NULL)
return FALSE;
fu_firmware_image_set_id (img_hdr, FU_FIRMWARE_IMAGE_ID_HEADER); fu_firmware_image_set_id (img_hdr, FU_FIRMWARE_IMAGE_ID_HEADER);
fu_firmware_image_set_bytes (img_hdr, fw_hdr); fu_firmware_image_set_bytes (img_hdr, fw_hdr);
fu_firmware_add_image (firmware, img_hdr); fu_firmware_add_image (firmware, img_hdr);
/* add payload */ /* add payload */
fw_payload = g_bytes_new_from_bytes (fw, sizeof(FuEbitdoFirmwareHeader), payload_len); fw_payload = fu_common_bytes_new_offset (fw,
sizeof(FuEbitdoFirmwareHeader),
payload_len,
error);
if (fw_payload == NULL)
return FALSE;
fu_firmware_image_set_id (img_payload, FU_FIRMWARE_IMAGE_ID_PAYLOAD); fu_firmware_image_set_id (img_payload, FU_FIRMWARE_IMAGE_ID_PAYLOAD);
fu_firmware_image_set_addr (img_payload, GUINT32_FROM_LE(hdr->destination_addr)); fu_firmware_image_set_addr (img_payload, GUINT32_FROM_LE(hdr->destination_addr));
fu_firmware_image_set_bytes (img_payload, fw_payload); fu_firmware_image_set_bytes (img_payload, fw_payload);

View File

@ -604,9 +604,12 @@ fu_thunderbolt_device_write_data (FuThunderboltDevice *self,
do { do {
g_autoptr(GBytes) fw_data = NULL; g_autoptr(GBytes) fw_data = NULL;
fw_data = g_bytes_new_from_bytes (blob_fw, fw_data = fu_common_bytes_new_offset (blob_fw,
nwritten, nwritten,
fw_size - nwritten); fw_size - nwritten,
error);
if (fw_data == NULL)
return FALSE;
n = g_output_stream_write_bytes (os, n = g_output_stream_write_bytes (os,
fw_data, fw_data,

View File

@ -356,10 +356,10 @@ fu_uefi_device_fixup_firmware (FuDevice *device, GBytes *fw, GError **error)
/* ESRT header matches payload */ /* ESRT header matches payload */
if (g_strcmp0 (fu_uefi_device_get_guid (self), guid_new) == 0) { if (g_strcmp0 (fu_uefi_device_get_guid (self), guid_new) == 0) {
g_debug ("ESRT matches payload GUID"); g_debug ("ESRT matches payload GUID");
return g_bytes_new_from_bytes (fw, 0, fw_length); return g_bytes_ref (fw);
/* Type that doesn't require a header */ /* Type that doesn't require a header */
} else if (!self->requires_header) { } else if (!self->requires_header) {
return g_bytes_new_from_bytes (fw, 0, fw_length); return g_bytes_ref (fw);
/* Missing, add a header */ /* Missing, add a header */
} else { } else {
guint header_size = getpagesize(); guint header_size = getpagesize();