Bug 1665371 (CVE-2019-3813) - CVE-2019-3813 spice: Off-by-one error in array access in spice/server/memslot.c
Summary: CVE-2019-3813 spice: Off-by-one error in array access in spice/server/memslot.c
Keywords:
Status: CLOSED ERRATA
Alias: CVE-2019-3813
Product: Security Response
Classification: Other
Component: vulnerability
Version: unspecified
Hardware: All
OS: Linux
high
high
Target Milestone: ---
Assignee: Red Hat Product Security
QA Contact:
URL:
Whiteboard:
Depends On: 1666830 1666831 1669226 1670158 1670159 1672153 1672154
Blocks: 1663867
TreeView+ depends on / blocked
 
Reported: 2019-01-11 07:35 UTC by Andrej Nemec
Modified: 2019-09-29 15:05 UTC (History)
23 users (show)

Fixed In Version: spice 0.14.2
Doc Type: If docs needed, set a value
Doc Text:
Spice, versions 0.5.2 through 0.14.1, are vulnerable to an out-of-bounds read due to an off-by-one error in memslot_get_virt. This may lead to a denial of service, or, in the worst case, code-execution by unauthenticated attackers.
Clone Of:
Environment:
Last Closed: 2019-01-31 20:08:31 UTC


Attachments (Terms of Use)


Links
System ID Priority Status Summary Last Updated
Red Hat Product Errata RHSA-2019:0231 None None None 2019-01-31 18:20:06 UTC
Red Hat Product Errata RHSA-2019:0232 None None None 2019-01-31 18:20:25 UTC
Red Hat Product Errata RHSA-2019:0457 None None None 2019-03-05 11:09:22 UTC

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.

Comment 19 errata-xmlrpc 2019-01-31 18:20:05 UTC
This issue has been addressed in the following products:

  Red Hat Enterprise Linux 7

Via RHSA-2019:0231 https://access.redhat.com/errata/RHSA-2019:0231

Comment 20 errata-xmlrpc 2019-01-31 18:20:24 UTC
This issue has been addressed in the following products:

  Red Hat Enterprise Linux 6

Via RHSA-2019:0232 https://access.redhat.com/errata/RHSA-2019:0232

Comment 26 errata-xmlrpc 2019-03-05 11:09:21 UTC
This issue has been addressed in the following products:

  Red Hat Virtualization 4 for Red Hat Enterprise Linux 7

Via RHSA-2019:0457 https://access.redhat.com/errata/RHSA-2019:0457


Note You need to log in before you can comment on or make changes to this bug.