Skip to content

Commit

Permalink
sscanf: Fix integer overflow with sscanf field width
Browse files Browse the repository at this point in the history
Fixes 5380975 ("sscanf: don't ignore field widths for numeric
conversions"). sscanf overflows integers with simple strings such as
dates. As an example, consider

	int r = sscanf("20190523123456", "%4d%2d%2d%2d%2d%2d",
		&year, &month, &day,
		&hour, &minute, &second);

	printk("%d %04d-%02d-%2d %02d:%02d:%02d\n",
		r,
		year, month, day,
		hour, minute, second);

On a 32-bit machine this prints

	6 0000-05-23 12:34:56

where the year is zero, and not 2019 as expected. The reason is that
sscanf attempts to read 20190523123456 as a whole integer, and then
divide it with 10^10 to obtain 2019, which obviously fails. Of course,
64-bit machines fail similarly on longer numerical strings.

The idea for a fix is to have a variant of _parse_integer() called
_parse_integer_end(), with the ability to stop consuming digits. The
functions

	simple_{strtol,strtoll,strtoul,strtoull}()

now have the corresponding

	sscanf_{strtol,strtoll,strtoul,strtoull}()

taking a field width into account. Perhaps this fix could be a starting-
point to clean-up the integer parsers? Also, it seems to be a good idea
to make a test suite for sscanf.
  • Loading branch information
frno7 committed Sep 7, 2019
1 parent a7d02f2 commit d3638dc
Show file tree
Hide file tree
Showing 3 changed files with 56 additions and 18 deletions.
10 changes: 8 additions & 2 deletions lib/kstrtox.c
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,15 @@ const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
*
* Don't you dare use this function.
*/
unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long *p)
unsigned int _parse_integer_end(const char *s, const char *e,
unsigned int base, unsigned long long *p)
{
unsigned long long res;
unsigned int rv;

res = 0;
rv = 0;
while (1) {
while (!e || s < e) {
unsigned int c = *s;
unsigned int lc = c | 0x20; /* don't tolower() this line */
unsigned int val;
Expand Down Expand Up @@ -82,6 +83,11 @@ unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long
return rv;
}

unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long *p)
{
return _parse_integer_end(s, NULL, base, p);
}

static int _kstrtoull(const char *s, unsigned int base, unsigned long long *res)
{
unsigned long long _res;
Expand Down
2 changes: 2 additions & 0 deletions lib/kstrtox.h
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

#define KSTRTOX_OVERFLOW (1U << 31)
const char *_parse_integer_fixup_radix(const char *s, unsigned int *base);
unsigned int _parse_integer_end(const char *s, const char *e,
unsigned int base, unsigned long long *p);
unsigned int _parse_integer(const char *s, unsigned int base, unsigned long long *res);

#endif
62 changes: 46 additions & 16 deletions lib/vsprintf.c
Original file line number Diff line number Diff line change
Expand Up @@ -123,6 +123,48 @@ long long simple_strtoll(const char *cp, char **endp, unsigned int base)
}
EXPORT_SYMBOL(simple_strtoll);

static unsigned long long sscanf_strtoull(const char *cp, int field_width,
char **endp, unsigned int base)
{
const char *e = field_width > 0 ? &cp[field_width] : NULL;
unsigned long long result;
unsigned int rv;

cp = _parse_integer_fixup_radix(cp, &base);
rv = _parse_integer_end(cp, e, base, &result);
/* FIXME */
cp += (rv & ~KSTRTOX_OVERFLOW);

if (endp)
*endp = (char *)cp;

return result;
}

static unsigned long sscanf_strtoul(const char *cp, int field_width,
char **endp, unsigned int base)
{
return sscanf_strtoull(cp, field_width, endp, base);
}

static long sscanf_strtol(const char *cp, int field_width,
char **endp, unsigned int base)
{
if (*cp == '-')
return -sscanf_strtoul(cp + 1, field_width - 1, endp, base);

return sscanf_strtoul(cp, field_width, endp, base);
}

static long long sscanf_strtoll(const char *cp, int field_width,
char **endp, unsigned int base)
{
if (*cp == '-')
return -sscanf_strtoull(cp + 1, field_width - 1, endp, base);

return sscanf_strtoull(cp, field_width, endp, base);
}

static noinline_for_stack
int skip_atoi(const char **s)
{
Expand Down Expand Up @@ -3330,24 +3372,12 @@ int vsscanf(const char *buf, const char *fmt, va_list args)

if (is_sign)
val.s = qualifier != 'L' ?
simple_strtol(str, &next, base) :
simple_strtoll(str, &next, base);
sscanf_strtol(str, field_width, &next, base) :
sscanf_strtoll(str, field_width, &next, base);
else
val.u = qualifier != 'L' ?
simple_strtoul(str, &next, base) :
simple_strtoull(str, &next, base);

if (field_width > 0 && next - str > field_width) {
if (base == 0)
_parse_integer_fixup_radix(str, &base);
while (next - str > field_width) {
if (is_sign)
val.s = div_s64(val.s, base);
else
val.u = div_u64(val.u, base);
--next;
}
}
sscanf_strtoul(str, field_width, &next, base) :
sscanf_strtoull(str, field_width, &next, base);

switch (qualifier) {
case 'H': /* that's 'hh' in format */
Expand Down

0 comments on commit d3638dc

Please sign in to comment.