mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
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);


  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®