CVE-2019-3813: Off-by-one error in array access in spice/server/memslot.c

Description Andrej Nemec 2019-01-11 07:35:53 UTC

An off-by-one error was found in spice when accessing arrays. A malicious guest user can use this for a host denial of service.

Comment 4 Scott Gayou 2019-01-14 23:47:03 UTC

Does look like it can change program control flow assuming checks can be bypassed, an attacker can control the heap information past the end of the buffer, etc. memslot_get_virt looks to return chunks of memory back to callers that are then cast into different structs and used. Thus, information leaks may be possible, lots of other super-undefined behavior could potentially occur (i.e. code execution)

``` │1426 qxl = (QXLCursor *)memslot_get_virt(slots, addr, sizeof(*qxl), group_id, &error); │

│1427 if (error) { │ │1428 return false; │ │1429 } │ │1430 │ │1431 red->header.unique = qxl->header.unique; │ │1432 red->header.type = qxl->header.type; │ ```

another example of a use of memslot_get_virt:

``` │1463 bool red_get_cursor_cmd(RedMemSlotInfo *slots, int group_id, │ │1464 RedCursorCmd *red, QXLPHYSICAL addr) │ │1465 { │ │1466 QXLCursorCmd *qxl; │ │1467 int error; │ │1468 │

│1469 qxl = (QXLCursorCmd *)memslot_get_virt(slots, addr, sizeof(*qxl), group_id, &error); │ │1470 if (error) { │ │1471 return false; │ │1472 } │ │1473 red->release_info_ext.info = &qxl->release_info; │ │1474 red->release_info_ext.group_id = group_id;
```

For an attacker to control the output memslot_get_virt, it looks like they need at least the following to occur:

1. Control group_id or the QXLPHYSICAL addr. Reporter indicated group_id may be unlikely, but QXLPHYSICAL might be possible.
2. If addr is controlled, set higher byte to slot_id such that “return addr >> info->memslot_id_shift” returns a number equal to info->num_memslots.
3. Control the heap such that the now out-of-bounds read of &info->mem_slots[group_id][slot_id] allows some control of the structure. Attacker would then potentially be able to modify the following struct:

```c typedef struct MemSlot { int generation; unsigned long virt_start_addr; unsigned long virt_end_addr; long address_delta; } MemSlot; ```

4. slot->generation would need to be set to what memslot_get_generation returns, which I think is just more information encoded into the addr.
5. h_virt could then potentially be set to anything via: ``` h_virt += slot->address_delta; ```
6. memslot_validate_virt would need to be bypassed. With full control of the heap, I believe that wouldn’t pose a problem. See the following line: ```c if (virt < slot->virt_start_addr || (virt + add_size) > slot->virt_end_addr) { ```

After all of these hypotheticals, an attacker could then return a controlled void* memslot_validate_virt to callers, who would then start processing those chunks of memory leading to further decay.

That’s my argument for potential (not easy, but possible) code execution. Highly likely I missed something, let me know. :)

Comment 6 Scott Gayou 2019-01-16 18:54:08 UTC

Also looks like an attacker may be able to change the group_id via modifying the QXL guest driver.

```c void spiceqxl_display_monitors_config(qxl_screen_t *qxl) { spice_qxl_monitors_config_async(&qxl->display_sin, physical_address(qxl, qxl->monitors_config, 0), MEMSLOT_GROUP, 0); } ``` MEMSLOT_GROUP is hardcoded to 0. So I believe an attacker can control this? Untested.

Comment 7 Scott Gayou 2019-01-16 19:01:19 UTC

Original commit – a70110c4e50aad99de7a844bb78eb868768e7841 on Sat Nov 21 22:42:38 2009 +0200

```bash git tag --contains a70110c4e50aad99de7a844bb78eb868768e7841 | sort -V 0.5.2 0.5.3 0.6.0 0.6.1 0.6.2 0.6.3 0.6.4 0.7.0 0.7.1 0.7.2 0.7.3 0.8.0 0.8.1 0.8.2 0.8.3 0.9.0 0.9.1 0.10.0 0.10.1 0.11.0 0.11.3 0.12.0 v0.12.0 v0.12.2 v0.12.3 v0.12.4 v0.12.5 v0.12.6 v0.12.8 v0.13.0 v0.13.1 v0.13.2 v0.13.3 v0.13.90 v0.13.91 v0.14.0 v0.14.1 ```

Thus, RHEL6 (spice-server 12.4) also impacted via the get_virt function in red_memslots.c.

Comment 8 Christophe Fergeau 2019-01-17 09:55:44 UTC

(In reply to Scott Gayou from comment #6) > Also looks like an attacker may be able to change the group_id via modifying

the QXL guest driver.

```c void spiceqxl_display_monitors_config(qxl_screen_t *qxl) { spice_qxl_monitors_config_async(&qxl->display_sin, physical_address(qxl, qxl->monitors_config, 0), MEMSLOT_GROUP, 0); } ``` MEMSLOT_GROUP is hardcoded to 0. So I believe an attacker can control this? Untested.

spiceqxl_*.c is not part of the QXL driver which runs on the guest, it’s a xorg driver which will directly link with spice-server, and will allow to export a xorg display over SPICE. I’m fairly confident group_id is never obtained from guest data, but is always hardcoded to a valid value by QEMU.

Comment 9 Scott Gayou 2019-01-17 19:25:31 UTC

Acknowledgments:

Name: Christophe Fergeau (Red Hat)

Comment 16 Scott Gayou 2019-01-28 18:37:31 UTC

Created spice tracking bugs for this issue:

Affects: fedora-all [bug 1670158]

Comment 18 Scott Gayou 2019-01-28 18:38:40 UTC

Unembargoed.