gvariant-parser: add assert to ensure we don't write too far

This function is complicated and it's hard to assess whether it is
correct or not, so let's add an assert just to be sure.

Notably, when writing this I initially had an off-by-one error in my
assert, causing the assertion to actually fire when running the unit
tests. So we know it's definitely possible for the function to use the
entirety of the buffer. The upper bound is not loose, and writing one
additional byte would be a security bug.

Note my assertion possibly doesn't protect against security issues
because it will be hit after the bad write rather than before.

Fixes #3469
This commit is contained in:
Michael Catanzaro 2024-09-19 09:59:13 -05:00
parent 1adc303f47
commit 7f4300c5da

View File

@ -431,6 +431,7 @@ pattern_coalesce (const gchar *left,
{ {
gchar *result; gchar *result;
gchar *out; gchar *out;
size_t buflen;
/* the length of the output is loosely bound by the sum of the input /* the length of the output is loosely bound by the sum of the input
* lengths, not simply the greater of the two lengths. * lengths, not simply the greater of the two lengths.
@ -439,7 +440,8 @@ pattern_coalesce (const gchar *left,
* *
* 8 + 8 = 12 * 8 + 8 = 12
*/ */
out = result = g_malloc (strlen (left) + strlen (right)); buflen = strlen (left) + strlen (right);
out = result = g_malloc (buflen);
while (*left && *right) while (*left && *right)
{ {
@ -493,6 +495,9 @@ pattern_coalesce (const gchar *left,
} }
} }
/* Need at least one byte remaining for trailing nul. */
g_assert (out < result + buflen);
if (*left || *right) if (*left || *right)
{ {
g_free (result); g_free (result);