linux-kernel - RE: [PATCH] lib: vsprintf: Avoid 32-bit truncation in vsscanf number parsing

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1fd034ae48b342429896120cefee91d9@AcuMS.aculab.com>
Date:   Thu, 12 Nov 2020 16:45:46 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Petr Mladek' <pmladek@...e.com>,
        Richard Fitzgerald <rf@...nsource.cirrus.com>
CC:     "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "sergey.senozhatsky@...il.com" <sergey.senozhatsky@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "patches@...nsource.cirrus.com" <patches@...nsource.cirrus.com>,
        "Andy Shevchenko" <andriy.shevchenko@...ux.intel.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>
Subject: RE: [PATCH] lib: vsprintf: Avoid 32-bit truncation in vsscanf number
 parsing

From: Petr Mladek
> Sent: 12 November 2020 16:18
> 
> Adding other vsprintf maintainers and reviewes into CC.
> 
> On Thu 2020-11-12 11:17:59, Richard Fitzgerald wrote:
> > Number conversion in vsscanf converts a whole string of digits and then
> > extracts the field width part from the converted value. The maximum run
> > of digits is limited by overflow. Conversion was using either
> > simple_strto[u]l or simple_strto[u]ll based on the 'L' qualifier. This
> > created a difference in truncation between builds where long is 32-bit
> > and builds where it is 64-bit. This especially affects parsing a run of
> > contiguous digits into separate fields - the maximum length of the run
> > is 16 digits if long is 64-bit but only 8 digits if long is 32-bits.
> > For example a conversion "%6x%6x" would convert both fields correctly if
> > long is 64-bit but not if long is 32-bit.

If %6x%6x works one might expect %6x%6x%6x to also work.

> I might be just slow today. But it took me really long time to
> understand what exactly is the problem and how it is caused.
> The description is nicely detailed but somehow cryptic.
> 
> My understanding is that there is a bug when parsing numbers
> with a limited width like the above mentioned "%6x%6x".
> 
> The problem is how the width is handled:
> 
>  1. The width is ignored in the 1st step. The entire number
>     is read using simple_strto[u]l[l] functions.
> 
>  2. The width limit is achieved by dividing the result from
>     the first step until it fits the width.
> 
> It gives wrong result when there was an overflow in the 1st step.
> The high bits were lost even when the limited number would
> not overflow.

What happens if there are leading zeros on the hex input?
>From the description I think 'something terrible happens'.
(Well, horribly unexpected anyway.)

I'd also expect strtoull() to 'eat' all the digits that exist,
not stop when the value got too large.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)