Fix use after free in serialization

Resolves: rhbz#1992873
Signed-off-by: Victor Toso <victortoso@redhat.com>
This commit is contained in:
Victor Toso 2021-09-15 18:42:15 +02:00
parent edf7814299
commit 62dc28ca77
2 changed files with 83 additions and 3 deletions

View File

@ -0,0 +1,74 @@
From 3b7e51c005a55f89f8a2e168853a96674ea52c11 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <public@hansmi.ch>
Date: Sun, 8 Aug 2021 15:35:58 +0200
Subject: [PATCH 1/1] Avoid use-after-free in serialization
Serializing parsers with large amounts of buffered write data (e.g. in case of
a slow or blocked write destination) would cause "serialize_data" to reallocate
the state buffer whose default size is 64kB (USBREDIRPARSER_SERIALIZE_BUF_SIZE).
The pointer to the position for the write buffer count would then point to
a location outside the buffer where the number of write buffers would be written
as a 32-bit value.
As of QEMU 5.2.0 the serializer is invoked for migrations. Serializations for
migrations may happen regularily such as when using the COLO feature[1].
Serialization happens under QEMU's I/O lock. The guest can't control the state
while the serialization is happening. The value written is the number of
outstanding buffers which would be suceptible to timing and host system system
load. The guest would have to continously groom the write buffers. A useful
value needs to be allocated in the exact position freed during the buffer size
increase, but before the buffer count is written. The author doesn't consider it
realistic to exploit this use-after-free reliably.
[1] https://wiki.qemu.org/Features/COLO
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
(cherry picked from commit 03c519ff5831ba75120e00ebebbf1d5a1f7220ab)
---
usbredirparser/usbredirparser.c | 8 +++++---
1 file changed, 5 insertions(+), 3 deletions(-)
diff --git a/usbredirparser/usbredirparser.c b/usbredirparser/usbredirparser.c
index 8f239cc..edfdfd2 100644
--- a/usbredirparser/usbredirparser.c
+++ b/usbredirparser/usbredirparser.c
@@ -20,6 +20,7 @@
*/
#include "config.h"
+#include <stddef.h>
#include <stdio.h>
#include <stdlib.h>
#include <stdarg.h>
@@ -1581,8 +1582,9 @@ int usbredirparser_serialize(struct usbredirparser *parser_pub,
struct usbredirparser_priv *parser =
(struct usbredirparser_priv *)parser_pub;
struct usbredirparser_buf *wbuf;
- uint8_t *write_buf_count_pos, *state = NULL, *pos = NULL;
+ uint8_t *state = NULL, *pos = NULL;
uint32_t write_buf_count = 0, len, remain = 0;
+ ptrdiff_t write_buf_count_pos;
*state_dest = NULL;
*state_len = 0;
@@ -1627,7 +1629,7 @@ int usbredirparser_serialize(struct usbredirparser *parser_pub,
parser->data, parser->data_read, "packet-data"))
return -1;
- write_buf_count_pos = pos;
+ write_buf_count_pos = pos - state;
/* To be replaced with write_buf_count later */
if (serialize_int(parser, &state, &pos, &remain, 0, "write_buf_count"))
return -1;
@@ -1642,7 +1644,7 @@ int usbredirparser_serialize(struct usbredirparser *parser_pub,
wbuf = wbuf->next;
}
/* Patch in write_buf_count */
- memcpy(write_buf_count_pos, &write_buf_count, sizeof(int32_t));
+ memcpy(state + write_buf_count_pos, &write_buf_count, sizeof(int32_t));
/* Patch in length */
len = pos - state;
--
2.31.1

View File

@ -1,12 +1,14 @@
Name: usbredir
Version: 0.8.0
Release: 8%{?dist}
Release: 9%{?dist}
Summary: USB network redirection protocol libraries
License: LGPLv2+
URL: http://spice-space.org/page/UsbRedir
Source0: http://spice-space.org/download/%{name}/%{name}-%{version}.tar.bz2
BuildRequires: make
Patch0001: 0001-Avoid-use-after-free-in-serialization.patch
BuildRequires: make
BuildRequires: gcc
BuildRequires: git-core
BuildRequires: libusb1-devel >= 1.0.9
%description
@ -43,7 +45,7 @@ A simple USB host TCP server, using libusbredirhost.
%prep
%setup -q
%autosetup -S git_am
%build
@ -78,6 +80,10 @@ rm $RPM_BUILD_ROOT%{_libdir}/libusbredir*.la
%changelog
* Wed Sep 15 2021 Victor Toso <victortoso@redhat.com> - 0.8.0-9
- Avoid use-after-free in serialization
Related: rhbz#1992873
* Tue Aug 10 2021 Mohan Boddu <mboddu@redhat.com> - 0.8.0-8
- Rebuilt for IMA sigs, glibc 2.34, aarch64 flags
Related: rhbz#1991688