69 lines
2.6 KiB
Diff
69 lines
2.6 KiB
Diff
|
From 33d9ba2fcc907b4f9a6c0540f9976b64b6f59db2 Mon Sep 17 00:00:00 2001
|
|||
|
|
From: Philip Withnall <pwithnall@gnome.org>
|
|||
|
|
Date: Tue, 3 Jun 2025 11:31:04 +0100
|
|||
|
|
Subject: [PATCH] gstring: Fix overflow check when expanding the string
|
|||
|
|
MIME-Version: 1.0
|
|||
|
|
Content-Type: text/plain; charset=UTF-8
|
|||
|
|
Content-Transfer-Encoding: 8bit
|
|||
|
|
|
|||
|
|
After commit 34b7992fd6e3894bf6d2229b8aa59cac34bcb1b5 the overflow check
|
|||
|
|
was only done when expanding the string, but we need to do it before
|
|||
|
|
checking whether to expand the string, otherwise that calculation could
|
|||
|
|
overflow and falsely decide that the string is big enough already.
|
|||
|
|
|
|||
|
|
As a concrete example, consider a `GString` which has:
|
|||
|
|
* `.len = G_MAXSIZE / 2 + 1`
|
|||
|
|
* `.allocated_len = G_MAXSIZE / 2 + 1`
|
|||
|
|
and `g_string_append()` is called on it with an input string of length
|
|||
|
|
`G_MAXSIZE / 2`.
|
|||
|
|
|
|||
|
|
This results in a call `g_string_maybe_expand (string, G_MAXSIZE / 2)`,
|
|||
|
|
which calculates `string->len + len` as `(G_MAXSIZE / 2 + 1) +
|
|||
|
|
(G_MAXSIZE / 2)` which evaluates to `1` as it overflows. This is not
|
|||
|
|
greater than `string->allocated_len` (which is `G_MAXSIZE / 2 + 1`), so
|
|||
|
|
`g_string_expand()` is *not* called, and `g_string_maybe_expand()`
|
|||
|
|
returns successfully. The caller then assumes that there’s enough space
|
|||
|
|
in the buffer, and happily continues to cause a buffer overflow.
|
|||
|
|
|
|||
|
|
It’s unlikely anyone could hit this in practice because it requires
|
|||
|
|
ludicrously big strings and `GString` allocations, which likely would
|
|||
|
|
have been blocked by other code, but if we’re going to have the overflow
|
|||
|
|
checks in `GString` then they should be effective.
|
|||
|
|
|
|||
|
|
Spotted by code inspection.
|
|||
|
|
|
|||
|
|
Signed-off-by: Philip Withnall <pwithnall@gnome.org>
|
|||
|
|
---
|
|||
|
|
glib/gstring.c | 8 ++++----
|
|||
|
|
1 file changed, 4 insertions(+), 4 deletions(-)
|
|||
|
|
|
|||
|
|
diff --git a/glib/gstring.c b/glib/gstring.c
|
|||
|
|
index 645746c9a..010a8e976 100644
|
|||
|
|
--- a/glib/gstring.c
|
|||
|
|
+++ b/glib/gstring.c
|
|||
|
|
@@ -68,10 +68,6 @@ static void
|
|||
|
|
g_string_expand (GString *string,
|
|||
|
|
gsize len)
|
|||
|
|
{
|
|||
|
|
- /* Detect potential overflow */
|
|||
|
|
- if G_UNLIKELY ((G_MAXSIZE - string->len - 1) < len)
|
|||
|
|
- g_error ("adding %" G_GSIZE_FORMAT " to string would overflow", len);
|
|||
|
|
-
|
|||
|
|
string->allocated_len = g_nearest_pow (string->len + len + 1);
|
|||
|
|
/* If the new size is bigger than G_MAXSIZE / 2, only allocate enough
|
|||
|
|
* memory for this string and don't over-allocate.
|
|||
|
|
@@ -86,6 +82,10 @@ static inline void
|
|||
|
|
g_string_maybe_expand (GString *string,
|
|||
|
|
gsize len)
|
|||
|
|
{
|
|||
|
|
+ /* Detect potential overflow */
|
|||
|
|
+ if G_UNLIKELY ((G_MAXSIZE - string->len - 1) < len)
|
|||
|
|
+ g_error ("adding %" G_GSIZE_FORMAT " to string would overflow", len);
|
|||
|
|
+
|
|||
|
|
if (G_UNLIKELY (string->len + len >= string->allocated_len))
|
|||
|
|
g_string_expand (string, len);
|
|||
|
|
}
|
|||
|
|
--
|
|||
|
|
2.49.0
|
|||
|
|
|