Re: [PATCH RFC] sscanf: Fix integer overflow with sscanf field width

[Jan Beulich]

>>> On 23.05.19 at 19:27, <noring@xxxxxxxxxx> wrote:
> This fixes 53809751ac23 ("sscanf: don't ignore field widths for numeric
> conversions"). sscanf overflows integers with simple strings such as dates.
> As an example, consider
> 
> 	r = sscanf("20190523123456", "%4d%2d%2d%2d%2d%2d",
> 		&year, &month, &day,
> 		&hour, &minute, &second);
> 
> 	pr_info("%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.

Right, and that's also what that commit's description says remains as
non-conforming behavior.

> I'm offering a simple patch to correct this below. The idea 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. There are some code duplication issues
> etc. so one might consider making more extensive changes than these.

I'm not the maintainer here, but to me it looks mostly okay.

> +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);

I'm afraid you may neither convert a field width of zero to -1 here,
nor convert a field width of 1 to zero (unlimited).

I'd also like to note that the 'u' and 'x' format characters also accept
a sign as per the standard, but that's an orthogonal issue which you
may or may not want to address at the same time.

Jan