From abb1e8d10b4751eeeb7304c77cead828d248094b Mon Sep 17 00:00:00 2001 From: Paul Smith Date: Mon, 6 Sep 2021 00:14:57 -0400 Subject: [PATCH] Remove UBSAN issues discovered via fuzzing tests. The arithmetic conversions in C say that if a binary operator has an unsigned and signed type as operands and the unsigned type has a greater rank then the signed value is converted to unsigned. This is bad if the signed value is negative. There are a few places in the code which have this situation; convert the signed value to positive and add instead of subtracting. Reported by He Jingxuan * src/read.c (find_map_unquote): Use a positive int in memmove(). (find_char_unquote): Ditto. (find_percent_cached): Ditto. --- src/read.c | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/src/read.c b/src/read.c index 65052fc3..4ef94c9a 100644 --- a/src/read.c +++ b/src/read.c @@ -2354,8 +2354,12 @@ find_map_unquote (char *string, int stopmap) string_len = strlen (string); /* The number of backslashes is now -I. Copy P over itself to swallow half of them. */ - memmove (&p[i], &p[i/2], (string_len - (p - string)) - (i/2) + 1); - p += i/2; + { + /* Avoid arithmetic conversion of negative values to unsigned. */ + int hi = -(i/2); + memmove (&p[i], &p[i/2], (string_len - (p - string)) + hi + 1); + p += i/2; + } if (i % 2 == 0) /* All the backslashes quoted each other; the STOPCHAR was unquoted. */ @@ -2397,8 +2401,12 @@ find_char_unquote (char *string, int stop) string_len = strlen (string); /* The number of backslashes is now -I. Copy P over itself to swallow half of them. */ - memmove (&p[i], &p[i/2], (string_len - (p - string)) - (i/2) + 1); - p += i/2; + { + /* Avoid arithmetic conversion of negative values to unsigned. */ + int hi = -(i/2); + memmove (&p[i], &p[i/2], (string_len - (p - string)) + hi + 1); + p += i/2; + } if (i % 2 == 0) /* All the backslashes quoted each other; the STOPCHAR was unquoted. */ @@ -2523,8 +2531,12 @@ find_percent_cached (const char **string) /* The number of backslashes is now -I. Copy P over itself to swallow half of them. */ - memmove (&pv[i], &pv[i/2], (slen - (pv - new)) - (i/2) + 1); - p += i/2; + { + /* Avoid arithmetic conversion of negative values to unsigned. */ + int hi = -(i/2); + memmove (&pv[i], &pv[i/2], (slen - (pv - new)) + hi + 1); + p += i/2; + } /* If the backslashes quoted each other; the % was unquoted. */ if (i % 2 == 0)