[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[PULL 23/37] hw/core/loader: In gunzip(), check index is in range before
From: |
Peter Maydell |
Subject: |
[PULL 23/37] hw/core/loader: In gunzip(), check index is in range before use, not after |
Date: |
Thu, 26 Aug 2021 18:02:53 +0100 |
The gunzip() function reads various fields from a passed in source
buffer in order to skip a header before passing the actual compressed
data to the zlib inflate() function. It does check whether the
passed in buffer is too small, but unfortunately it checks that only
after reading bytes from the src buffer, so it could read off the end
of the buffer.
You can see this with valgrind:
$ printf "%b" '\x1f\x8b' > /tmp/image
$ valgrind qemu-system-aarch64 -display none -M virt -cpu max -kernel
/tmp/image
[...]
==19224== Invalid read of size 1
==19224== at 0x67302E: gunzip (loader.c:558)
==19224== by 0x673907: load_image_gzipped_buffer (loader.c:788)
==19224== by 0xA18032: load_aarch64_image (boot.c:932)
==19224== by 0xA18489: arm_setup_direct_kernel_boot (boot.c:1063)
==19224== by 0xA18D90: arm_load_kernel (boot.c:1317)
==19224== by 0x9F3651: machvirt_init (virt.c:2114)
==19224== by 0x794B7A: machine_run_board_init (machine.c:1272)
==19224== by 0xD5CAD3: qemu_init_board (vl.c:2618)
==19224== by 0xD5CCA6: qmp_x_exit_preconfig (vl.c:2692)
==19224== by 0xD5F32E: qemu_init (vl.c:3713)
==19224== by 0x5ADDB1: main (main.c:49)
==19224== Address 0x3802a873 is 0 bytes after a block of size 3 alloc'd
==19224== at 0x4C31B0F: malloc (in
/usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
==19224== by 0x61E7657: g_file_get_contents (in
/usr/lib/x86_64-linux-gnu/libglib-2.0.so.0.5600.4)
==19224== by 0x673895: load_image_gzipped_buffer (loader.c:771)
==19224== by 0xA18032: load_aarch64_image (boot.c:932)
==19224== by 0xA18489: arm_setup_direct_kernel_boot (boot.c:1063)
==19224== by 0xA18D90: arm_load_kernel (boot.c:1317)
==19224== by 0x9F3651: machvirt_init (virt.c:2114)
==19224== by 0x794B7A: machine_run_board_init (machine.c:1272)
==19224== by 0xD5CAD3: qemu_init_board (vl.c:2618)
==19224== by 0xD5CCA6: qmp_x_exit_preconfig (vl.c:2692)
==19224== by 0xD5F32E: qemu_init (vl.c:3713)
==19224== by 0x5ADDB1: main (main.c:49)
Check that we have enough bytes of data to read the header bytes that
we read before we read them.
Fixes: Coverity 1458997
Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
Message-id: 20210812141803.20913-1-peter.maydell@linaro.org
---
hw/core/loader.c | 35 +++++++++++++++++++++++++----------
1 file changed, 25 insertions(+), 10 deletions(-)
diff --git a/hw/core/loader.c b/hw/core/loader.c
index 5b34869a541..c623318b737 100644
--- a/hw/core/loader.c
+++ b/hw/core/loader.c
@@ -555,24 +555,35 @@ ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
size_t srclen)
/* skip header */
i = 10;
+ if (srclen < 4) {
+ goto toosmall;
+ }
flags = src[3];
if (src[2] != DEFLATED || (flags & RESERVED) != 0) {
puts ("Error: Bad gzipped data\n");
return -1;
}
- if ((flags & EXTRA_FIELD) != 0)
+ if ((flags & EXTRA_FIELD) != 0) {
+ if (srclen < 12) {
+ goto toosmall;
+ }
i = 12 + src[10] + (src[11] << 8);
- if ((flags & ORIG_NAME) != 0)
- while (src[i++] != 0)
- ;
- if ((flags & COMMENT) != 0)
- while (src[i++] != 0)
- ;
- if ((flags & HEAD_CRC) != 0)
+ }
+ if ((flags & ORIG_NAME) != 0) {
+ while (i < srclen && src[i++] != 0) {
+ /* do nothing */
+ }
+ }
+ if ((flags & COMMENT) != 0) {
+ while (i < srclen && src[i++] != 0) {
+ /* do nothing */
+ }
+ }
+ if ((flags & HEAD_CRC) != 0) {
i += 2;
+ }
if (i >= srclen) {
- puts ("Error: gunzip out of data in header\n");
- return -1;
+ goto toosmall;
}
s.zalloc = zalloc;
@@ -596,6 +607,10 @@ ssize_t gunzip(void *dst, size_t dstlen, uint8_t *src,
size_t srclen)
inflateEnd(&s);
return dstbytes;
+
+toosmall:
+ puts("Error: gunzip out of data in header\n");
+ return -1;
}
/* Load a U-Boot image. */
--
2.20.1
- [PULL 13/37] docs/specs/acpi_nvdimm: Convert to rST, (continued)
- [PULL 13/37] docs/specs/acpi_nvdimm: Convert to rST, Peter Maydell, 2021/08/26
- [PULL 11/37] docs/specs/acpi_mem_hotplug: Convert to rST, Peter Maydell, 2021/08/26
- [PULL 16/37] monitor: Use accel_find("kvm") instead of kvm_available(), Peter Maydell, 2021/08/26
- [PULL 17/37] softmmu/arch_init.c: Trim down include list, Peter Maydell, 2021/08/26
- [PULL 19/37] arch_init.h: Add QEMU_ARCH_HEXAGON, Peter Maydell, 2021/08/26
- [PULL 20/37] arch_init.h: Move QEMU_ARCH_VIRTIO_* to qdev-monitor.c, Peter Maydell, 2021/08/26
- [PULL 18/37] meson.build: Define QEMU_ARCH in config-target.h, Peter Maydell, 2021/08/26
- [PULL 22/37] stubs: Remove unused arch_type.c stub, Peter Maydell, 2021/08/26
- [PULL 21/37] arch_init.h: Don't include arch_init.h unnecessarily, Peter Maydell, 2021/08/26
- [PULL 24/37] softmmu/physmem.c: Remove unneeded NULL check in qemu_ram_alloc_from_fd(), Peter Maydell, 2021/08/26
- [PULL 23/37] hw/core/loader: In gunzip(), check index is in range before use, not after,
Peter Maydell <=
- [PULL 26/37] net: Zero sockaddr_in in parse_host_port(), Peter Maydell, 2021/08/26
- [PULL 30/37] raspi: Use error_fatal for SoC realize errors, not error_abort, Peter Maydell, 2021/08/26
- [PULL 28/37] tests/qtest/ipmi-bt-test: Zero-initialize sockaddr struct, Peter Maydell, 2021/08/26
- [PULL 32/37] hw/arm/virt: Delete EL3 error checksnow provided in CPU realize, Peter Maydell, 2021/08/26
- [PULL 27/37] gdbstub: Zero-initialize sockaddr structs, Peter Maydell, 2021/08/26
- [PULL 29/37] tests/tcg/multiarch/linux-test: Zero-initialize sockaddr structs, Peter Maydell, 2021/08/26
- [PULL 25/37] softmmu/physmem.c: Check return value from realpath(), Peter Maydell, 2021/08/26
- [PULL 33/37] target/arm: Implement HSTR.TTEE, Peter Maydell, 2021/08/26
- [PULL 36/37] hw/arm/xlnx-versal: Add unimplemented APU mmio, Peter Maydell, 2021/08/26
- [PULL 35/37] target/arm: Do hflags rebuild in cpsr_write(), Peter Maydell, 2021/08/26