Skip to content

Commit

Permalink
Fix buffer overflow in do_composition()
Browse files Browse the repository at this point in the history
I tried to verify that Linux 6.2's decision to use -funsigned-char would
break our code and was mostly successful, with the sole exception of
do_composition(), which has a bug that -funsigned-char would have
lessened, although not mitigated.

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 iterate at least SIZE_MAX-1
times, which will massively overflow the buffer, which is not fine. This would
undoubtably cause horrible memory corruption, which would likely crash the
system, but could cause just about anything until the system crashes.

Signed-off-by: Richard Yao <richard.yao@alumni.stonybrook.edu>
  • Loading branch information
ryao committed Dec 23, 2022
1 parent 918e8df commit ce11d11
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 ce11d11

Please sign in to comment.