QEMU: use after free vulnerability in esp_do_dma() in hw/scsi/esp.c

[Mauro Matteo Cascella]

A use-after-free vulnerability was found in the am53c974 SCSI host bus adapter emulation of QEMU. It could occur in the esp_do_dma() function in hw/scsi/esp.c while handling the 'Information Transfer' command (CMD_TI). A privileged guest user may abuse this flaw to crash the QEMU process on the host, resulting in a denial of service or potential code execution with the privileges of the QEMU process.

This issue was reported by Cheolwoo Myung (Seoul National University).

Original report:
Using hypervisor fuzzer, hyfuzz, I found a use-after-free issue in
am53c974 emulator of QEMU enabled ASan.

It occurs while transferring information, as it does not check the
buffer to be transferred.

A malicious guest user/process could use this flaw to crash the QEMU
process resulting in DoS scenario.

To reproduce this issue, please run the QEMU with the following command
line.

# To enable ASan option, please set configuration with the following
$ ./configure --target-list=i386-softmmu --disable-werror --enable-sanitizers
$ make

# To reproduce this issue, please run the QEMU process with the following command line
$ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \
-device am53c974,id=scsi -device scsi-hd,drive=SysDisk \
-drive id=SysDisk,if=none,file=./disk.img

Please find attached the disk images to reproduce this issue.

[Mauro Matteo Cascella (mauro-cascella)]

RHBZ: https://bugzilla.redhat.com/show_bug.cgi?id=1909996

[Alexander Bulekov (a1xndr)]

Looks the same, or very similar to this one:
/*
 * Autogenerated Fuzzer Test Case
 *
 * This work is licensed under the terms of the GNU GPL, version 2 or
 * later. See the COPYING file in the top-level directory.
 */

#include "qemu/osdep.h"

#include "libqos/libqtest.h"

/*
 * cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, \
 * -m 4G -device am53c974,id=scsi -device scsi-hd,drive=disk0 -drive \
 * id=disk0,if=none,file=null-co://,format=raw -nodefaults -qtest stdio
 * outl 0xcf8 0x80001010
 * outl 0xcfc 0xc000
 * outl 0xcf8 0x80001004
 * outw 0xcfc 0x01
 * outl 0xc046 0x02
 * outl 0xc03f 0x0300
 * outw 0xc00b 0x4300
 * outl 0xc00b 0x9000
 * EOF
 */
static void test_fuzz(void)
{
    QTestState *s = qtest_init(
        "-display none , -m 4G -device am53c974,id=scsi -device "
        "scsi-hd,drive=disk0 -drive "
        "id=disk0,if=none,file=null-co://,format=raw -nodefaults");
    qtest_outl(s, 0xcf8, 0x80001010);
    qtest_outl(s, 0xcfc, 0xc000);
    qtest_outl(s, 0xcf8, 0x80001004);
    qtest_outw(s, 0xcfc, 0x01);
    qtest_outl(s, 0xc046, 0x02);
    qtest_outl(s, 0xc03f, 0x0300);
    qtest_outw(s, 0xc00b, 0x4300);
    qtest_outl(s, 0xc00b, 0x9000);
    qtest_quit(s);
}
int main(int argc, char **argv)
{
    const char *arch = qtest_get_arch();

    g_test_init(&argc, &argv, NULL);

    if (strcmp(arch, "i386") == 0) {
        qtest_add_func("fuzz/test_fuzz", test_fuzz);
    }

    return g_test_run();
}

[Mauro Matteo Cascella (mauro-cascella)]

Technically, the first one is a heap use-after-free, while the second a stack buffer overflow. They could be two different manifestations of the same issue; they both originate from handle_ti() and the root cause may be the same.

Heap uaf:
=================================================================
==129653==ERROR: AddressSanitizer: heap-use-after-free on address 0x6290000b5000 at pc 0x7f0c3d947dd3 bp 0x7f0c13bfdac0 sp 0x7f0c13bfd270
READ of size 27 at 0x6290000b5000 thread T7
    #0 0x7f0c3d947dd2 in __interceptor_memcpy (/lib64/libasan.so.6+0x39dd2)
    #1 0x562c1c7292b2 in flatview_write_continue softmmu/physmem.c:2781
    #2 0x562c1c729589 in flatview_write softmmu/physmem.c:2816
    #3 0x562c1c729ef7 in address_space_write softmmu/physmem.c:2908
    #4 0x562c1c729faf in address_space_rw softmmu/physmem.c:2918
    #5 0x562c1c217754 in dma_memory_rw_relaxed include/sysemu/dma.h:8
    #6 0x562c1c2177a1 in dma_memory_rw include/sysemu/dma.h:127
    #7 0x562c1c21791b in pci_dma_rw include/hw/pci/pci.h:803
    #8 0x562c1c21b6e3 in esp_pci_dma_memory_rw hw/scsi/esp-pci.c:283
    #9 0x562c1c21ba6e in esp_pci_dma_memory_write hw/scsi/esp-pci.c:302
    #10 0x562c1c428685 in esp_do_dma hw/scsi/esp.c:526
    #11 0x562c1c429cb5 in handle_ti hw/scsi/esp.c:629
    ...

Stack bof:
=================================================================
==138588==ERROR: AddressSanitizer: stack-buffer-overflow on address 0x7ffc8a90c300 at pc 0x559b1de0780e bp 0x7ffc8a90bd10 sp 0x7ffc8a90bd08
WRITE of size 4 at 0x7ffc8a90c300 thread T0
    #0 0x559b1de0780d in stl_he_p include/qemu/bswap.h:353
    #1 0x559b1de07dec in stn_he_p include/qemu/bswap.h:486
    #2 0x559b1de23e47 in flatview_read_continue softmmu/physmem.c:2841
    #3 0x559b1de24215 in flatview_read softmmu/physmem.c:2879
    #4 0x559b1de243b5 in address_space_read_full softmmu/physmem.c:2892
    #5 0x559b1de2462c in address_space_rw softmmu/physmem.c:2920
    #6 0x559b1d1ec514 in dma_memory_rw_relaxed include/sysemu/dma.h:88
    #7 0x559b1d1ec561 in dma_memory_rw include/sysemu/dma.h:127
    #8 0x559b1d1ec6db in pci_dma_rw include/hw/pci/pci.h:803
    #9 0x559b1d1f04a3 in esp_pci_dma_memory_rw hw/scsi/esp-pci.c:283
    #10 0x559b1d1f07f8 in esp_pci_dma_memory_read hw/scsi/esp-pci.c:296
    #11 0x559b1d66fab1 in esp_do_dma hw/scsi/esp.c:576
    #12 0x559b1d6746e1 in handle_ti hw/scsi/esp.c:845
    ...

[Mauro Matteo Cascella (mauro-cascella)]

Note that the use-after-free was found in v5.2.0 and, as far as I can tell, is not reproducible anymore on master. The ESP/NCR53C9x emulator (hw/scsi/esp.c) underwent several changes since v5.2.0. By git-bisecting, it looks like the original reproducer is neutralized after commit [1]. However, the qtest reproducer (comment #3) seems to be working fine on master as of today.

[1] https://git.qemu.org/?p=qemu.git;a=commit;h=bb0bc7bbc9764a5e9e81756819838c5db88652b8

[Alexander Bulekov (a1xndr)]

Hi Mauro,
Oops... I missed that it was a stack-overflow. I went through my list of crashes, and the closest one I can find is a heap UAF, but it is a write, rather than a read:

/*
 * Autogenerated Fuzzer Test Case
 *
 * Copyright (c) 2021 <name of author>
 *
 * This work is licensed under the terms of the GNU GPL, version 2 or
 * later. See the COPYING file in the top-level directory.
 */

#include "qemu/osdep.h"

#include "libqos/libqtest.h"

/*
 * cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, \
 * -m 4G -device am53c974,id=scsi -device scsi-hd,drive=disk0 -drive \
 * id=disk0,if=none,file=null-co://,format=raw -nodefaults -qtest stdio
 * outl 0xcf8 0x80001010
 * outl 0xcfc 0xc000
 * outl 0xcf8 0x80001004
 * outw 0xcfc 0x05
 * outb 0xc046 0x02
 * outl 0xc00b 0xc100
 * outl 0xc040 0x03
 * outl 0xc040 0x03
 * write 0x0 0x1 0x41
 * outl 0xc00b 0xc100
 * outw 0xc040 0x02
 * outl 0xc00b 0x9000
 * EOF
 */
static void test_fuzz(void)
{
    QTestState *s = qtest_init(
        "-display none , -m 4G -device am53c974,id=scsi -device "
        "scsi-hd,drive=disk0 -drive "
        "id=disk0,if=none,file=null-co://,format=raw -nodefaults");
    qtest_outl(s, 0xcf8, 0x80001010);
    qtest_outl(s, 0xcfc, 0xc000);
    qtest_outl(s, 0xcf8, 0x80001004);
    qtest_outw(s, 0xcfc, 0x05);
    qtest_outb(s, 0xc046, 0x02);
    qtest_outl(s, 0xc00b, 0xc100);
    qtest_outl(s, 0xc040, 0x03);
    qtest_outl(s, 0xc040, 0x03);
    qtest_bufwrite(s, 0x0, "\x41", 0x1);
    qtest_outl(s, 0xc00b, 0xc100);
    qtest_outw(s, 0xc040, 0x02);
    qtest_outl(s, 0xc00b, 0x9000);
    qtest_quit(s);
}
int main(int argc, char **argv)
{
    const char *arch = qtest_get_arch();

    g_test_init(&argc, &argv, NULL);

    if (strcmp(arch, "i386") == 0) {
        qtest_add_func("fuzz/test_fuzz", test_fuzz);
    }

    return g_test_run();
}

[Mark Cave-Ayland (mark-cave-ayland)]

Thank you both for the reproducers. Please see the proposed patchset here:

https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg06063.html

[P J P (pjps)]

On Wednesday, 17 March, 2021, 10:26:36 pm IST, Cheolwoo Myung <email address hidden> wrote:
> Hello PJP, Mauro
>
> Of course. you can post the details with our reproducers.
> I'm glad it helped you.
>
> Thank you.
> - Cheolwoo Myung
>

2021년 3월 17일 (수) 오후 10:30, P J P <email address hidden>님이 작성:
>
>On Monday, 15 March, 2021, 07:54:30 pm IST, Mauro Matteo Cascella <email address hidden> wrote:
>>JFYI, CVE-2020-35506 was assigned to a very similar (if not the same)
>>issue, see https://bugs.launchpad.net/qemu/+bug/1909247.
>
> * From the QEMU command lines below they do look similar.
>
> * CVE bug above does not link to an upstream fix/patch. Maybe it's not fixed yet?
>
>
>On Mon, Mar 15, 2021 at 6:58 AM P J P <email address hidden> wrote:
> >On Monday, 15 March, 2021, 11:11:14 am IST, Cheolwoo Myung <email address hidden> wrote:
> >Using hypervisor fuzzer, hyfuzz, I found a use-after-free issue in am53c974 emulator of QEMU enabled ASan.
> >
> ># To reproduce this issue, please run the QEMU process with the following command line.
> >$ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \
> > -device am53c974,id=scsi -device scsi-hd,drive=SysDisk -drive >id=SysDisk,if=none,file=./disk.img
> >
> >
> > Using hypervisor fuzzer, hyfuzz, I found a stack buffer overflow issue in am53c974 emulator of QEMU enabled ASan.
> >
> ># To reproduce this issue, please run the QEMU process with the following command line.
> >$ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \
> > -device am53c974,id=scsi -device scsi-hd,drive=SysDisk -drive >id=SysDisk,if=none,file=./disk.img
> >

* I was able to reproduce these issues against the latest upstream git source
  and following patch helps to fix above two issues.
===
$ git diff hw/scsi/
diff --git a/hw/scsi/esp-pci.c b/hw/scsi/esp-pci.c
index c3d3dab05e..4a6f208069 100644
--- a/hw/scsi/esp-pci.c
+++ b/hw/scsi/esp-pci.c
@@ -98,6 +98,7 @@ static void esp_pci_handle_abort(PCIESPState *pci, uint32_t val)
     trace_esp_pci_dma_abort(val);
     if (s->current_req) {
         scsi_req_cancel(s->current_req);
+ s->async_len = 0;
     }
 }

diff --git a/hw/scsi/esp.c b/hw/scsi/esp.c
index 507ab363bc..99bee7bc66 100644
--- a/hw/scsi/esp.c
+++ b/hw/scsi/esp.c
@@ -564,7 +564,7 @@ static void esp_do_dma(ESPState *s)
     int to_device = ((s->rregs[ESP_RSTAT] & 7) == STAT_DO);
     uint8_t buf[ESP_CMDFIFO_SZ];

- len = esp_get_tc(s);
+ len = MIN(esp_get_tc(s), sizeof(buf));
     if (s->do_cmd) {
         /*
===

> >Using hypervisor fuzzer, hyfuzz, I found a heap buffer overflow issue in am53c974 emulator of QEMU enabled ASan.
> >
> ># To reproduce this issue, please run the QEMU process with the following command line.
> >$ ./qemu-system-i386 -m 512 -drive file=./hyfuzz.img,index=0,media=disk,format=raw \
> > -device am53c974,id=scsi -device scsi-hd,drive=SysDisk -drive >id=SysDisk,if=none,file=./disk.img

* This heap OOB access issue seems to occur because

   static void do_busid_cmd(...)
     ...
     buf = (uint8_t *)fifo8_pop_buf(&s->cmdfifo, cmdlen, &n); <==

'buf' points towards an end of the 32 byte buffer allocated via

   static void e...

[Mark Cave-Ayland (mark-cave-ayland)]

Can you confirm that this is fixed in the v2 of the above patchset?

https://lists.gnu.org/archive/html/qemu-devel/2021-03/msg06550.html

ATB,

Mark.

[Mauro Matteo Cascella (mauro-cascella)]

Hello,

Thank you all for your comments. Both patches (PJP/comment#8 - Mark/comment#9) seem to properly fix the UAF reported by Alexander in comment #6. However, I'm still able to reproduce the heap-bof from the above hw-esp-oob-issues.zip:

./x86_64-softmmu/qemu-system-x86_64 -m 512 \
-drive file=./atch2/hyfuzz.img,index=0,media=disk,format=raw \
-device am53c974,id=scsi -device scsi-hd,drive=SysDisk \
-drive id=SysDisk,if=none,file=./atch2/disk.img

[Alexander Bulekov (a1xndr)]

Hi,
I can still trigger stack-overflows, heap-UAFs and heap-overflows in the
code, but Mark's patches fixed some of the issues. I didn't want to
flood the issue-tracker with further problems in this code, since it
isn't clear what the security expectations are for this device. Of
course it is only a matter of time until someone sends more reports to
qemu-security.

Mark, do you want me to provide more reproducers for this device?
-Alex

[Philippe Mathieu-Daudé (philmd)]

On 3/24/21 4:53 PM, Alexander Bulekov wrote:
> Hi,
> I can still trigger stack-overflows, heap-UAFs and heap-overflows in the
> code, but Mark's patches fixed some of the issues. I didn't want to
> flood the issue-tracker with further problems in this code, since it
> isn't clear what the security expectations are for this device. Of
> course it is only a matter of time until someone sends more reports to
> qemu-security.

I'd expect qemu-security to have a template "Thank you for your bug
but this device is not within the 'security' boundary, we will forward
your report to the community".

>
> Mark, do you want me to provide more reproducers for this device?

Surely Mark prefers you provide bugfixes instead :D

Phil.

[Mark Cave-Ayland (mark-cave-ayland)]

If Alex is interested in having a fuzz-proof device as a starting point for fuzzing QEMU's SCSI layer then I don't mind doing the basic work as I've spent a few months deep in the internals of the ESP controller, and it makes sense to look at this whilst it is all still fresh. I'd say there's at least one more set of ESP changes already waiting for after the 6.0 release.

PJP:
Your change to esp-pci.c looks like a genuine issue, although there is an inconsistency within ESP as to what determines whether a request is in progress or not. My v2 patchset above uses the request member being non-NULL to indicate a valid request, but this should be made consistent throughout the driver.

Can you provide a qtest reproducer so that it can be incorporated into the test included in the v2 patchset and also allow me to check that this issue has been fixed?

Alex:
If you can try PJP's patch to esp-pci.c and if you still see some issues then please update this bug with a test case or two, and I will look at them when I get a moment.

Mauro:
Thanks for the test case - again I shall look at this when I have some available time.

[Alexander Bulekov (a1xndr)]

Add some more regression tests for the esp device.

(Prasad's Patch)
Based-on: <email address hidden>
(Mark's v2 Patchset)
Based-on: <email address hidden>
Signed-off-by: Alexander Bulekov <email address hidden>
---

Hi Mark,
Hopefully these are useful. I realized that my previous message was
innacurate (I forgot to apply Prasad's patch, or your v2
patchset). The only corruptions that I am continuing to see are
heap-overflows. I am guessing that most of these are due to some mututal
root cause, so the number of tests far-exceeds the actual number of
errors, but I am providing all of the crashes with unique-looking
stack-traces, just in case.
Please let me know if I can provide anything else that would help.
-Alex

 tests/qtest/am53c974-test.c | 1137 +++++++++++++++++++++++++++++++++++
 1 file changed, 1137 insertions(+)

diff --git a/tests/qtest/am53c974-test.c b/tests/qtest/am53c974-test.c
index c90bd4c187..cb2a5646a6 100644
--- a/tests/qtest/am53c974-test.c
+++ b/tests/qtest/am53c974-test.c
@@ -9,6 +9,1125 @@

 #include "libqos/libqtest.h"

+/*
+ * cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
+ * 512M -device am53c974,id=scsi -device scsi-hd,drive=disk0 -drive \
+ * id=disk0,if=none,file=null-co://,format=raw -nodefaults -qtest stdio
+ * outl 0xcf8 0x80001010
+ * outl 0xcfc 0xc000
+ * outl 0xcf8 0x80001004
+ * outw 0xcfc 0x01
+ * outb 0xc000 0x4
+ * outb 0xc008 0xa0
+ * outl 0xc03f 0x0300
+ * outl 0xc00b 0xc300
+ * outw 0xc00b 0x9000
+ * outl 0xc00b 0xc300
+ * outl 0xc00b 0xc300
+ * outl 0xc00b 0xc300
+ * outw 0xc00b 0x9000
+ * outw 0xc00b 0x1000
+ * EOF
+ */
+static void crash_0900379669(void)
+{
+ QTestState *s = qtest_init(
+ "-display none -m 512M -device am53c974,id=scsi -device scsi-hd,drive=disk0 "
+ "-drive id=disk0,if=none,file=null-co://,format=raw -nodefaults");
+ qtest_outl(s, 0xcf8, 0x80001010);
+ qtest_outl(s, 0xcfc, 0xc000);
+ qtest_outl(s, 0xcf8, 0x80001004);
+ qtest_outw(s, 0xcfc, 0x01);
+ qtest_outb(s, 0xc000, 0x4);
+ qtest_outb(s, 0xc008, 0xa0);
+ qtest_outl(s, 0xc03f, 0x0300);
+ qtest_outl(s, 0xc00b, 0xc300);
+ qtest_outw(s, 0xc00b, 0x9000);
+ qtest_outl(s, 0xc00b, 0xc300);
+ qtest_outl(s, 0xc00b, 0xc300);
+ qtest_outl(s, 0xc00b, 0xc300);
+ qtest_outw(s, 0xc00b, 0x9000);
+ qtest_outw(s, 0xc00b, 0x1000);
+ qtest_quit(s);
+}
+/*
+ * cat << EOF | ./qemu-system-i386 -display none -machine accel=qtest, -m \
+ * 512M -device am53c974,id=scsi -device scsi-hd,drive=disk0 -drive \
+ * id=disk0,if=none,file=null-co://,format=raw -nodefaults -qtest stdio
+ * outl 0xcf8 0x80001010
+ * outl 0xcfc 0xc000
+ * outl 0xcf8 0x80001004
+ * outw 0xcfc 0x01
+ * outl 0xc008 0x20
+ * outw 0xc000 0x1
+ * outb 0xc040 0x03
+ * outl 0xc00b 0xc200
+ * outl 0xc00b 0xc200
+ * outl 0xc00b 0xc200
+ * outl 0xc00b 0xc200
+ * outl 0xc00b 0xc200
+ * outl 0xc00b 0xc200
+ * outl 0xc00b 0xc200
+ * outl 0xc00b 0xc200
+ * outl 0xc00b 0xc200
+ * outl 0xc00b 0xc200
+ * outl 0xc00b 0xc200
+ * outw 0xc00b 0x4200
+ * outl 0xc00a 0x410000
+ * EOF
+ */
+static void crash_094661a91b(void)
+{
+ QTestState ...

[Mark Cave-Ayland (mark-cave-ayland)]

Thanks again Alex. I've just posted a v3 to the list which fixes your extra test cases, and also those contained within the uaf and hw-esp-oob attachments:

https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg00015.html

[Mauro Matteo Cascella (mauro-cascella)]

This is fixed now, thank you Mark.

Patchset v4:
https://lists.gnu.org/archive/html/qemu-devel/2021-04/msg01000.html

Upstream commits:
https://git.qemu.org/?p=qemu.git;a=commit;h=0db895361b8a82e1114372ff9f48
https://git.qemu.org/?p=qemu.git;a=commit;h=e392255766071c8cac480da3a9ae
https://git.qemu.org/?p=qemu.git;a=commit;h=e5455b8c1c6170c788f3c0fd577c
https://git.qemu.org/?p=qemu.git;a=commit;h=c5fef9112b15c4b5494791cdf8bb
https://git.qemu.org/?p=qemu.git;a=commit;h=7b320a8e67a534925048cbabfa51
https://git.qemu.org/?p=qemu.git;a=commit;h=99545751734035b76bd372c4e721
https://git.qemu.org/?p=qemu.git;a=commit;h=fa7505c154d4d00ad89a747be2ed
https://git.qemu.org/?p=qemu.git;a=commit;h=fbc6510e3379fa8f8370bf71198f
https://git.qemu.org/?p=qemu.git;a=commit;h=0ebb5fd80589835153a0c2baa1b8
https://git.qemu.org/?p=qemu.git;a=commit;h=324c8809897c8c53ad05c3a7147d
https://git.qemu.org/?p=qemu.git;a=commit;h=607206948cacda4a80be5b976dba