---===[ Qubes Security Bulletin 089 ]===---

                             2023-05-11

       Qrexec: Memory corruption in service request handling

User action required
---------------------

Users must install the following specific packages in order to address
the issues discussed in this bulletin:

  For Qubes 4.1, in dom0:
  - qrexec packages, version 4.1.21

These packages will migrate from the security-testing repository to the
current (stable) repository over the next two weeks after being tested
by the community. [1] Once available, the packages are to be installed
via the Qubes Update tool or its command-line equivalents. [2]

Summary
--------

Due to a bug in qrexec [3], a malicious qube can cause memory corruption
in the qrexec-daemon. The Qubes Security Team is not aware of any way to
exploit this vulnerability in an attack (not even in a denial-of-service
attack that only crashes the process). However, we cannot completely
rule out such a possibility.

Impact
-------

While we consider the successful exploitation of this vulnerability to
be very unlikely, an attacker could theoretically use it to crash the
qrexec-daemon or execute arbitrary code in dom0.

Discussion
-----------

Qubes OS features a framework known as "qrexec," which allows different
qubes to communicate with each other in a controlled manner. [3][4]
These interactions are restricted by the system's RPC policies. [5] In
particular, qrexec can be used to allow less trusted qubes to
communicate with more trusted qubes, including dom0.

Incoming RPC calls are handled by the qrexec-daemon process. Qubes OS
4.1 introduced a new qrexec message type, `MSG_TRIGGER_SERVICE3`, which
allows much larger requests (theoretically up to 65000 bytes, compared
to 64 bytes in earlier versions). This message type uses a
dynamically-allocated buffer for the message body based on the request
length. The code used to validate the request length has an off-by-one
error that can cause memory corruption, as described below.

First, the incoming message is validated in the
`sanitize_message_from_agent()` function:

    1177 static void sanitize_message_from_agent(struct msg_header *untrusted_header)
    1178 {
    1179     switch (untrusted_header->type) {
    ...
    1191         case MSG_TRIGGER_SERVICE3:
    1192             if (protocol_version < QREXEC_PROTOCOL_V3) {
    1193                 LOG(ERROR, "agent sent (new) MSG_TRIGGER_SERVICE3 "
    1194                     "although it uses protocol %d", protocol_version);
    1195                 exit(1);
    1196             }
    1197             if (untrusted_header->len < sizeof(struct trigger_service_params3)) {
    1198                 LOG(ERROR, "agent sent invalid MSG_TRIGGER_SERVICE3 packet");
    1199                 exit(1);
    1200             }
    1201             if (untrusted_header->len - sizeof(struct trigger_service_params3)
    1202                     > MAX_SERVICE_NAME_LEN) {
    1203                 LOG(ERROR, "agent sent too large MSG_TRIGGER_SERVICE3 packet");
    1204                 exit(1);
    1205             }
    1206             break;
    ...

The second condition, on line 1197, verifies that the message sent by
the qrexec-agent (from a VM) is not shorter than the message header
defined in `struct trigger_service_params3`. However, it fails to
account for a mandatory NUL character at the end of the message payload
(the service name and its argument). Later on, the
`handle_message_from_agent()` function processes the message as follows:

    1222 static void handle_message_from_agent(void)
    1223 {
    1224     struct msg_header hdr, untrusted_hdr;
    1225     struct trigger_service_params untrusted_params, params;
    1226     struct trigger_service_params3 untrusted_params3, params3;
    1227     char *untrusted_service_name = NULL, *service_name = NULL;
    1228     size_t service_name_len;
    1229
    1230     if (libvchan_recv(vchan, &untrusted_hdr, sizeof(untrusted_hdr))
    1231             != sizeof(untrusted_hdr))
    1232         handle_vchan_error("recv hdr");
    1233     /* sanitize start */
    1234     sanitize_message_from_agent(&untrusted_hdr);
    1235     hdr = untrusted_hdr;
    1236     /* sanitize end */
    ...
    1241     switch (hdr.type) {
    ...
    1262         case MSG_TRIGGER_SERVICE3:
    1263             service_name_len = hdr.len - sizeof(untrusted_params3);
    1264             untrusted_service_name = malloc(service_name_len);
    1265             if (!untrusted_service_name)
    1266                 handle_vchan_error("malloc(service_name)");
    1267
    1268             if (libvchan_recv(vchan, &untrusted_params3, sizeof(untrusted_params3))
    1269                     != sizeof(untrusted_params3))
    1270                 handle_vchan_error("recv params3");
    1271             if (libvchan_recv(vchan, untrusted_service_name, service_name_len)
    1272                     != (int)service_name_len)
    1273                 handle_vchan_error("recv params3(service_name)");
    1274
    1275             /* sanitize start */
    1276             ENSURE_NULL_TERMINATED(untrusted_params3.target_domain);
    1277             ENSURE_NULL_TERMINATED(untrusted_params3.request_id.ident);
    1278             untrusted_service_name[service_name_len-1] = 0;
    1279             sanitize_name(untrusted_params3.target_domain, "@:");
    1280             sanitize_name(untrusted_params3.request_id.ident, " ");
    1281             sanitize_name(untrusted_service_name, "+");
    1282             params3 = untrusted_params3;
    1283             service_name = untrusted_service_name;
    1284             untrusted_service_name = NULL;
    1285             /* sanitize end */
    ...

The initial call to `sanitize_message_from_agent()` is visible in line
1234. Then, the function calculates the expected service name length in
line 1263, allocates memory for it in line 1264, and receives both the
rest of the header and its payload in lines 1268-1273. Since
`sanitize_message_from_agent()` allows the `hdr.len` to be equal to
`sizeof(untrusted_params3)`, `service_name_len` can be zero. This means
that adding the terminating NUL character in line 1278 can write outside
of the allocated buffer. Furthermore, in such a case, the
`untrusted_service_name` buffer is allocated with a size of zero, and
the `sanitize_name()` call in line 1281 can write beyond the buffer too.

The `sanitize_name()` function, listed below, replaces disallowed
characters with underscores (byte 0x5f) until it finds the terminating
NUL character:

    759 static void sanitize_name(char * untrusted_s_signed, char *extra_allowed_chars)
    760 {
    761     unsigned char * untrusted_s;
    762     for (untrusted_s=(unsigned char*)untrusted_s_signed; *untrusted_s; untrusted_s++) {
    763         if (*untrusted_s >= 'a' && *untrusted_s <= 'z')
    764             continue;
    765         if (*untrusted_s >= 'A' && *untrusted_s <= 'Z')
    766             continue;
    767         if (*untrusted_s >= '0' && *untrusted_s <= '9')
    768             continue;
    769         if (*untrusted_s == '_' ||
    770                *untrusted_s == '-' ||
    771                *untrusted_s == '.')
    772             continue;
    773         if (extra_allowed_chars && strchr(extra_allowed_chars, *untrusted_s))
    774             continue;
    775         *untrusted_s = '_';
    776     }
    777 }

This code runs in dom0. In Qubes OS 4.1, dom0 is based on Fedora 32
x86_64, which uses the GNU libc 2.31 library. In this implementation
[5], the `malloc()` call (line 1264) always allocates at least 32 bytes,
even if the requested size is 0. This means that the returned pointer is
not NULL in this case (which would be allowed by the specification for
zero-sized allocations), so the error handling in line 1266 does not
interrupt the processing. Consequently, the NUL character written in
line 1278 hits part of the malloc metadata, specifically the most
significant byte of the allocation size. Given that the size is small
(between the 32-byte `MIN_CHUNK_SIZE` and the 1024-byte
`MIN_LARGE_SIZE`), that byte is always zero already, so this write is
harmless.

The remaining concern is about the `sanitize_name()` function. For the
bug to be harmful, it must overwrite something beyond the allocated
buffer (at least 32 bytes). This means that there cannot be a zero byte
in this area. In our analysis, this is very unlikely to occur, for the
following reasons:

1. The `malloc()` call in `handle_message_from_agent()` is the one and
   only call that occurs outside of startup and connection setup. No
   other malloc call occurs in that process, not even an indirect one.
   The connection setup allocations can be redone when the qrexec-agent
   disconnects and connects again, but the points below also apply to
   this reconnection sequence. (Perhaps the only exception is handling
   an error message just before exit(1), but if the attacker hasn't
   corrupted malloc metadata up to this point, the attacker won't have a
   chance to do it after this point either.)

2. The qrexec-daemon handles only a single request at a time. At no
   point are multiple buffers for `service_name` allocated in the
   process at the same time. All remaining handling occurs in a separate
   process (which also handles only a single request).

3. Due to how GNU libc's malloc works, if there is a small chunk
   available, it will be used. It won't be split out of a bigger chunk.
   And, due to point 2 above, if there was a small chunk initially, it
   will get used again.

4. The allocations done during startup and reconnection leave a few
   small free chunks that are interleaved with allocated areas (the
   libvchan_t structure, something from libxengnttab, libxentoollog, and
   a few others). These memory chunks can't be merged due to their
   layout.

5. Every small allocation includes a NUL byte somewhere in the payload
   before being freed. This applies to both initial areas as well as to
   the buffers used for `service_name`.

Given all of these considerations, successfully exploiting this
vulnerability in an attack is very unlikely. However, due to the
complexity of the memory allocator, we cannot completely rule out such a
possibility.

Credits
--------

This issue was discovered by Demi Marie Obenour.

References
-----------

[1] https://www.qubes-os.org/doc/testing/
[2] https://www.qubes-os.org/doc/how-to-update/
[3] https://www.qubes-os.org/doc/qrexec/
[4] https://www.qubes-os.org/doc/qrexec-internals/
[5] https://sourceware.org/glibc/wiki/MallocInternals

--
The Qubes Security Team
https://www.qubes-os.org/security/

-----BEGIN PGP SIGNATURE-----

iQIzBAABCAAdFiEELRdx/k12ftx2sIn61lWk8hgw4GoFAmRcEs8ACgkQ1lWk8hgw
4GoOyg/+MotkJUbomFgs+lxw/oMhJ9kDAoDd678OeJyKn+7SQ2RUq+LlAvoHwqoS
FUNwb/g7ZzS4oBphpvxiXK1AoV8ZX00AjKaMgPgLAkWBQyx0zY0RCyQGELwf/j4r
iL+yClcRTcQ4VuJa4hicJjnxVD5xSwqUlqKz7fNDNPiFGTTqaymnHO7ZNR0OI4sM
Alx/BvJE8a8yBC1mW+hiO8TcfE33uQMhtE1N3rr5VxAKeIyfk0dW8iPGoLYECVBS
aV6YfOBoeKwlrZPgbuUhRIbPuWnhL7tJTttda1BF9H4RuS58UVcg2MHvkgrRKMqD
qLHWFB3S/CVPboGwiTlnBoss05HRMfkLDxTBeEO1UVEF6FfGgXoS1gFMrRZyvTEp
+T+90mCXD2QhpcvIqFvZBP/BW3pp4iVijs/EdrqZIBuV1zN1FRrTe02dy3XtIx0I
qGh+iamaQG43v+exPwQG0eYcBjjSxG9CRtDcCXixt0uh5qlT31upI9XD0e2E/1AM
SG6EFdMeKs5bYMNGnq2pDCOWBscE8DRK6gfj8ZEnPxPzuZY7vP3vrRo5QbM8l4Jx
NDu+kJolEaQXYP00ofKw3wcb6Rh2v/iW6PstpntxgWRVC2wOyzmhs4puXPzGfIaj
3nqkUovrQszbKJXKhZlNOnXT/oWVUmjj0Oy6o0Xrk20/0Y5E8Lk=
=1b+G
-----END PGP SIGNATURE-----

-----BEGIN PGP SIGNATURE-----

iQIzBAABCgAdFiEE6hjn8EDEHdrv6aoPSsGN4REuFJAFAmRcKqkACgkQSsGN4REu
FJChQxAAgSDk/FsVdd6pDi5p82s7/lsdnPdLs7pinoM7MY5tlC/Mi2+7Jd3CvZbf
ok/d3LmAt0/qs2rwKIrDwnTCxcrscTTT0q0QovRZQNUPGtAtgyGagHH22H/wLMxq
aheHXT5bV8K1GL+GIXfyIb0a2jFf62h1YcJklmgiHDiRIjv9rR/lNyCYm1PzSOC2
M/NruX3xoq15fQCZVar/0yxy35hynX38ZXYxfLvAV3VBZMBVpBz5QkXZ5namMhYr
XqRy1N5oTbQaTbNYbwYsG7uR4d0SOKevMlXvmAFVIYS0YlrLy0rGyOJAsoNiwHg2
z9h5eAAZKr68vTfOIa3JLRqRmpAmnQnG5POafhB8bJXCl/TFaPm2meFs19jiD8kE
zp6o2jb+Af5WcprSrDGW5JPmwuMQiV/+D/4JA8o5OKqmiE4iQosio/P7OP4eWlpP
H0BWxdpczYSBfBdHHMZzrHjcq1xssXsTg52W1JTJQ8QiDbSioTm0bi/ZF3C7ovsA
DN5rbLDdOJUwKbOroFuaj1snWS/UZ9AAM2rMMDJr3F///ErVC/DAyMFieIeTpkHN
EfVHstuherfMwWUFA6+3JrTsONx1t16svTw2coEYLxwhkIvuj61iiRFKpgMnDZf1
ssE+QXzSx+Zt4ee7b5m2ci2pFfaQZWabyk+5Z5GtmiWrMJS6FIM=
=zjXb
-----END PGP SIGNATURE-----

$ gpg --verify qsb-089-2023.txt.sig.marmarek qsb-089-2023.txt
$ gpg --verify qsb-089-2023.txt.sig.simon qsb-089-2023.txt