[PATCH v1 2/6] lib: scanf: handle integer overflows in vsscanf

From: Konstantin Khlebnikov
Date: Sun Mar 10 2019 - 12:57:01 EST


Traditional scanf implementations ignore integer overflows because
C language standard allows here undefined behavior (Â7.21.6.2 #10).

So, sane and safe behavior wouldn't harm anything.

This patch carefully checks integer overflows and stops matching if result
does not fit into appropriate type before assigning it into argument.

Signed-off-by: Konstantin Khlebnikov <khlebnikov@xxxxxxxxxxxxxx>
---
lib/vsprintf.c | 86 ++++++++++++++++++++++++++++++++++++--------------------
1 file changed, 56 insertions(+), 30 deletions(-)

diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 276a0bc3b019..ada0501f1525 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -3026,12 +3026,13 @@ EXPORT_SYMBOL_GPL(bprintf);
* - "%[...]" requires field width
* - %s without field width limited with SHRT_MAX
* - "%*..." simply skips non white-space characters without conversion
+ * - integer overflows are handled as matching failure
*/
int vsscanf(const char *buf, const char *fmt, va_list args)
{
const char *str = buf;
- char *next;
- char digit;
+ const char *next;
+ unsigned int rv;
int num = 0;
u8 qualifier;
unsigned int base;
@@ -3230,29 +3231,31 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
*/
str = skip_spaces(str);

- digit = *str;
- if (is_sign && digit == '-')
- digit = *(str + 1);
+ next = str;

- if (!digit
- || (base == 16 && !isxdigit(digit))
- || (base == 10 && !isdigit(digit))
- || (base == 8 && (!isdigit(digit) || digit > '7'))
- || (base == 0 && !isdigit(digit)))
- break;
+ /* skip leading sign */
+ if (is_sign && (*str == '+' || *str == '-'))
+ next++;

- if (is_sign)
- val.s = qualifier != 'L' ?
- simple_strtol(str, &next, base) :
- simple_strtoll(str, &next, base);
- else
- val.u = qualifier != 'L' ?
- simple_strtoul(str, &next, base) :
- simple_strtoull(str, &next, base);
+ /* 64-bit integer conversion, similar to _kstrtoull() */
+ next = _parse_integer_fixup_radix(next, &base);
+ rv = _parse_integer(next, base, &val.u);
+ if (rv == 0)
+ break;
+ if (rv & KSTRTOX_OVERFLOW)
+ goto overflow;
+ next += rv;
+
+ if (is_sign) {
+ if (*str == '-') {
+ val.s = -val.u;
+ if (val.s > 0)
+ goto overflow;
+ } else if (val.s < 0)
+ goto overflow;
+ }

- if (field_width > 0 && next - str > field_width) {
- if (base == 0)
- _parse_integer_fixup_radix(str, &base);
+ if (field_width > 0) {
while (next - str > field_width) {
if (is_sign)
val.s = div_s64(val.s, base);
@@ -3264,22 +3267,37 @@ int vsscanf(const char *buf, const char *fmt, va_list args)

switch (qualifier) {
case 'H': /* that's 'hh' in format */
- if (is_sign)
+ if (is_sign) {
+ if ((signed char)val.s != val.s)
+ goto overflow;
*va_arg(args, signed char *) = val.s;
- else
+ } else {
+ if ((unsigned char)val.u != val.u)
+ goto overflow;
*va_arg(args, unsigned char *) = val.u;
+ }
break;
case 'h':
- if (is_sign)
+ if (is_sign) {
+ if ((short)val.s != val.s)
+ goto overflow;
*va_arg(args, short *) = val.s;
- else
+ } else {
+ if ((unsigned short)val.u != val.u)
+ goto overflow;
*va_arg(args, unsigned short *) = val.u;
+ }
break;
case 'l':
- if (is_sign)
+ if (is_sign) {
+ if ((long)val.s != val.s)
+ goto overflow;
*va_arg(args, long *) = val.s;
- else
+ } else {
+ if ((unsigned long)val.u != val.u)
+ goto overflow;
*va_arg(args, unsigned long *) = val.u;
+ }
break;
case 'L':
if (is_sign)
@@ -3288,13 +3306,20 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
*va_arg(args, unsigned long long *) = val.u;
break;
case 'z':
+ if ((size_t)val.u != val.u)
+ goto overflow;
*va_arg(args, size_t *) = val.u;
break;
default:
- if (is_sign)
+ if (is_sign) {
+ if ((int)val.s != val.s)
+ goto overflow;
*va_arg(args, int *) = val.s;
- else
+ } else {
+ if ((unsigned int)val.u != val.u)
+ goto overflow;
*va_arg(args, unsigned int *) = val.u;
+ }
break;
}
num++;
@@ -3304,6 +3329,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
str = next;
}

+overflow:
return num;
}
EXPORT_SYMBOL(vsscanf);