From: Konstantin Khlebnikov <khlebnikov@yandex-team.ru>
To: linux-kernel@vger.kernel.org
Cc: Tejun Heo <tj@kernel.org>,
Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
Andrew Morton <akpm@linux-foundation.org>,
Linus Torvalds <torvalds@linux-foundation.org>,
Alexey Dobriyan <adobriyan@gmail.com>
Subject: [PATCH v1 2/6] lib: scanf: handle integer overflows in vsscanf
Date: Sun, 10 Mar 2019 19:56:32 +0300 [thread overview]
Message-ID: <155223699287.4075.13184771244455866778.stgit@buzz> (raw)
In-Reply-To: <155223448227.4075.6846910559654700796.stgit@buzz>
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@yandex-team.ru>
---
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);
next prev parent reply other threads:[~2019-03-10 16:57 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-03-10 16:56 [PATCH v1 0/6] Make sscanf safer Konstantin Khlebnikov
2019-03-10 16:56 ` [PATCH v1 1/6] lib: scanf: document features of scanf format string Konstantin Khlebnikov
2019-03-10 16:56 ` Konstantin Khlebnikov [this message]
2019-03-10 21:06 ` [PATCH v1 2/6] lib: scanf: handle integer overflows in vsscanf Rasmus Villemoes
2019-03-10 21:52 ` Linus Torvalds
2019-03-11 7:22 ` Konstantin Khlebnikov
2019-03-10 16:56 ` [PATCH v1 3/6] lib: scanf: add vsscanf feature for matching end of text Konstantin Khlebnikov
2019-03-10 16:56 ` [PATCH v1 4/6] lib: scanf: handle character ranges in %[...] Konstantin Khlebnikov
2019-03-10 16:56 ` [PATCH v1 5/6] lib: scanf: mark sscanf and vsscanf as __must_check Konstantin Khlebnikov
2019-03-10 16:56 ` [PATCH v1 6/6] lib: scanf: add test module Konstantin Khlebnikov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=155223699287.4075.13184771244455866778.stgit@buzz \
--to=khlebnikov@yandex-team.ru \
--cc=adobriyan@gmail.com \
--cc=akpm@linux-foundation.org \
--cc=gregkh@linuxfoundation.org \
--cc=linux-kernel@vger.kernel.org \
--cc=tj@kernel.org \
--cc=torvalds@linux-foundation.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®