forked from pool/libX11
Stefan Dirsch
b3ab4adca7
* fixes off-by-one write in XListExtensions (bsc#1102062, CVE-2018-14599) - u_out-of-boundary-write-in-XListExtensions.patch * fixes out of boundary write in XListExtensions (bsc#1102068, CVE-2018-14600) - u_crash-on-invalid-reply-in-XListExtensions.patch * crash on invalid reply in XListExtensions (bsc#1102073, CVE-2018-14598) OBS-URL: https://build.opensuse.org/package/show/X11:XOrg/libX11?expand=0&rev=45
68 lines
2.2 KiB
Diff
68 lines
2.2 KiB
Diff
From b4692168dfd66cdcd91d970ff255ded144d6ef95 Mon Sep 17 00:00:00 2001
|
|
From: Stefan Dirsch <sndirsch@suse.de>
|
|
Date: Mon, 23 Jul 2018 14:26:05 +0200
|
|
Subject: [PATCH] off-by-one write in XListExtensions
|
|
References: bsc#1102062 CVE-2018-14599
|
|
|
|
The function XListExtensions is vulnerable to an off-by-one override on
|
|
malicious server responses.
|
|
|
|
The server reply consists of extension names consisting of a length byte
|
|
followed by actual string, which is not NUL-terminated.
|
|
|
|
While parsing the response, the length byte is overridden with '\0',
|
|
thus the memory area can be used as storage of C strings later on. To
|
|
be able to NUL-terminate the last string, the buffer is reserved with
|
|
an additional byte of space.
|
|
|
|
For a boundary check, the variable chend (end of ch) was introduced,
|
|
pointing at the end of the buffer which ch initially points to.
|
|
Unfortunately there is a difference in handling "the end of ch".
|
|
|
|
While chend points at the first byte that must not be written to,
|
|
the for-loop uses chend as the last byte that can be written to.
|
|
|
|
Therefore, an off-by-one can occur.
|
|
|
|
I have refactored the code so chend actually points to the last byte
|
|
that can be written to without an out of boundary access. As it is not
|
|
possible to achieve "ch + length < chend" and "ch + length + 1 > chend"
|
|
with the corrected chend meaning, I removed the inner if-check.
|
|
|
|
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
|
|
---
|
|
src/ListExt.c | 12 ++++--------
|
|
1 file changed, 4 insertions(+), 8 deletions(-)
|
|
|
|
diff --git a/src/ListExt.c b/src/ListExt.c
|
|
index 7fdf9932..8f344ac0 100644
|
|
--- a/src/ListExt.c
|
|
+++ b/src/ListExt.c
|
|
@@ -74,19 +74,15 @@ char **XListExtensions(
|
|
/*
|
|
* unpack into null terminated strings.
|
|
*/
|
|
- chend = ch + (rlen + 1);
|
|
+ chend = ch + rlen;
|
|
length = *ch;
|
|
for (i = 0; i < rep.nExtensions; i++) {
|
|
if (ch + length < chend) {
|
|
list[i] = ch+1; /* skip over length */
|
|
ch += length + 1; /* find next length ... */
|
|
- if (ch <= chend) {
|
|
- length = *ch;
|
|
- *ch = '\0'; /* and replace with null-termination */
|
|
- count++;
|
|
- } else {
|
|
- list[i] = NULL;
|
|
- }
|
|
+ length = *ch;
|
|
+ *ch = '\0'; /* and replace with null-termination */
|
|
+ count++;
|
|
} else
|
|
list[i] = NULL;
|
|
}
|
|
--
|
|
2.16.4
|
|
|