Skip to content

Commit

Permalink
Fix buffer overflow in do_composition()
Browse files Browse the repository at this point in the history
In do_composition(), we have:

size = u8_number_of_bytes[*p];
if (size <= 1 || (p + size) > oslast)
    break;

There, we have type promotion from char to size_t, which is unsigned. C
will sign extend the value as part of the widening before treating the
value as unsigned and the negative values we can counter are error
values from U8_ILLEGAL_CHAR and U8_OUT_OF_RANGE_CHAR, which are -1 and
-2 respectively. The unsigned versions of these under two's complement
are SIZE_MAX and SIZE_MAX-1 respectively.

The bounds check is written under the assumption that `size <= 1` does a
signed comparison. This is followed by a pointer comparison to see if
the string has the correct length, which is fine.

A little further down we have:

for (i = 0; i < size; i++)
    tc[i] = *p++;

When an error condition is encountered, this will attempt to iterate at
least SIZE_MAX-1 times, which will massively overflow the buffer, which
is not fine.

The kernel will kill the loop as soon as it hits the kernel stack guard
on Linux systems built with CONFIG_VMAP_STACK=y, which should be just
about all of them. That prevents arbitrary code execution and just about
any other bad thing that a black hat attacker might attempt with
knowledge of this buffer overflow. Other systems' kernels have
mitigations for unbounded in-kernel buffer overflows that will catch
this too.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
  • Loading branch information
ryao committed Dec 27, 2022
1 parent c935fe2 commit f67912a
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion module/unicode/u8_textprep.c
Original file line number Diff line number Diff line change
Expand Up @@ -1287,7 +1287,7 @@ do_composition(size_t uv, uchar_t *s, uchar_t *comb_class, uchar_t *start,

while (p < oslast) {
size = u8_number_of_bytes[*p];
if (size <= 1 || (p + size) > oslast)
if ((ssize_t)size <= 1 || (p + size) > oslast)
break;

saved_p = p;
Expand Down

0 comments on commit f67912a

Please sign in to comment.