usbredir/0006-Avoid-memory-leak-from-ill-formatted-serialization-d.patch
2021-12-24 16:19:25 +08:00

77 lines
2.9 KiB
Diff

From babd82e4f83d479e9fb418cd2dd55be6a95a1fe7 Mon Sep 17 00:00:00 2001
From: Michael Hanselmann <public@hansmi.ch>
Date: Thu, 26 Aug 2021 22:32:55 +0200
Subject: [PATCH] Avoid memory leak from ill-formatted serialization data
At commit 060d7914ea the following `fuzzing/usbredirparserfuzz` input triggers
a memory leak:
```
$ base64 -d <<'EOF' | gunzip -c > testcase
H4sIAFDwJ2ECA2NgZIADBQWF/zD2HQgfAv4DgcJ/BTzgP0iRAlwRivL/ePX9
R1VAyBaSjcShB8nPTBA9/xn+g5UAANvH+dkSAQAA
```
The data type header segment is empty while there's (supposed) payload data in
there. The internal buffers must be filled in the order of header, type header
and then data, an invariant important for `usbredirparser_do_read` and violated
by this input.
With this change the input data is read the same way, but if the invariant would
be violated the data read is just ignored.
The parser check at the beginning of `usbredirparser_unserialize` is also
improved and `write_buf_count` is no longer set explicitly.
Signed-off-by: Michael Hanselmann <public@hansmi.ch>
---
usbredirparser/usbredirparser.c | 18 +++++++++++++-----
1 file changed, 13 insertions(+), 5 deletions(-)
diff --git a/usbredirparser/usbredirparser.c b/usbredirparser/usbredirparser.c
index c158c66..a0312fb 100644
--- a/usbredirparser/usbredirparser.c
+++ b/usbredirparser/usbredirparser.c
@@ -1692,8 +1692,9 @@ int usbredirparser_unserialize(struct usbredirparser *parser_pub,
return -1;
}
- if (parser->write_buf_count != 0 || parser->write_buf != NULL ||
- parser->data != NULL) {
+ if (!(parser->write_buf_count == 0 && parser->write_buf == NULL &&
+ parser->data == NULL && parser->header_read == 0 &&
+ parser->type_header_read == 0 && parser->data_read == 0)) {
ERROR("unserialization must use a pristine parser");
return -1;
}
@@ -1769,7 +1770,9 @@ int usbredirparser_unserialize(struct usbredirparser *parser_pub,
i = parser->type_header_len;
if (unserialize_data(parser, &state, &remain, &data, &i, "type_header"))
return -1;
- parser->type_header_read = i;
+ if (parser->header_read == header_len) {
+ parser->type_header_read = i;
+ }
if (parser->data_len) {
parser->data = malloc(parser->data_len);
@@ -1781,8 +1784,13 @@ int usbredirparser_unserialize(struct usbredirparser *parser_pub,
i = parser->data_len;
if (unserialize_data(parser, &state, &remain, &parser->data, &i, "data"))
return -1;
- parser->data_read = i;
- parser->write_buf_count = 0;
+ if (parser->header_read == header_len &&
+ parser->type_header_read == parser->type_header_len) {
+ parser->data_read = i;
+ } else if (parser->data != NULL) {
+ free(parser->data);
+ parser->data = NULL;
+ }
/* Get the write buffer count and the write buffers */
if (unserialize_int(parser, &state, &remain, &i, "write_buf_count"))
--
1.8.3.1