Hi Thomas, On 9/25/19 3:03 PM, Thomas Huth wrote: > Both, "rom->addr" and "addr" are derived from the binary image > that can be loaded with the "-kernel" paramer. The code in > rom_copy() then calculates: > > d = dest + (rom->addr - addr); > > and uses "d" as destination in a memcpy() some lines later. Now with > bad kernel images, it is possible that rom->addr is smaller than addr, > thus "rom->addr - addr" gets negative and the memcpy() then tries to > copy contents from the image to a bad memory location. In the best case, > this just crashes QEMU, in the worst case, this could maybe be used to > inject code from the kernel image into the QEMU binary, so we better fix > it with an additional sanity check here. > > Cc: qemu-sta...@nongnu.org > Reported-by: Guangming Liu > Buglink: https://bugs.launchpad.net/qemu/+bug/1844635
"This page does not exist, or you may not have permission to see it." This seems security related. Shouldn't we open a CVE for this? https://wiki.qemu.org/SecurityProcess#CVE_allocation Let's say I have write access to a LAN TFTP server used by some PXE bootloader where I can store my crafted nasty kernel, then I get this score: https://nvd.nist.gov/vuln-metrics/cvss/v3-calculator?vector=AV:A/AC:L/PR:N/UI:N/S:C/C:H/I:H/A:H/E:P/RL:O/RC:C&version=3.1 CVSS Base Score: 9.6 CVSS Temporal Score: 8.6 Which seems quite high. > Signed-off-by: Thomas Huth <th...@redhat.com> > --- > hw/core/loader.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/hw/core/loader.c b/hw/core/loader.c > index 0d60219364..5099f27dc8 100644 > --- a/hw/core/loader.c > +++ b/hw/core/loader.c > @@ -1281,7 +1281,7 @@ int rom_copy(uint8_t *dest, hwaddr addr, size_t size) $ git show 235f86ef014 Date: Thu Nov 12 21:53:11 2009 +0100 This function is old and poorly documented. > if (rom->addr + rom->romsize < addr) { > continue; > } > - if (rom->addr > end) { > + if (rom->addr > end || rom->addr < addr) { Reviewed-by: Philippe Mathieu-Daudé <phi...@redhat.com> > break; > } > >
