linux-kernel - RE: [PATCH v3 3/4] vsscanf(): do not skip spaces

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: <0c8bb9a3d2de4cf5b06f912d784e7292@AcuMS.aculab.com>
Date:   Mon, 12 Jun 2023 11:11:11 +0000
From:   David Laight <David.Laight@...LAB.COM>
To:     'Demi Marie Obenour' <demi@...isiblethingslab.com>,
        Hans de Goede <hdegoede@...hat.com>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        "Sakari Ailus" <sakari.ailus@...ux.intel.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Juergen Gross <jgross@...e.com>,
        "Stefano Stabellini" <sstabellini@...nel.org>,
        Oleksandr Tyshchenko <oleksandr_tyshchenko@...m.com>,
        Lee Jones <lee@...nel.org>, Andy Lutomirski <luto@...nel.org>,
        Thomas Gleixner <tglx@...utronix.de>,
        Vincenzo Frascino <vincenzo.frascino@....com>,
        Petr Mladek <pmladek@...e.com>,
        Steven Rostedt <rostedt@...dmis.org>,
        Sergey Senozhatsky <senozhatsky@...omium.org>,
        "Andy Shevchenko" <andriy.shevchenko@...ux.intel.com>,
        Rasmus Villemoes <linux@...musvillemoes.dk>
CC:     "linux-media@...r.kernel.org" <linux-media@...r.kernel.org>,
        "linux-staging@...ts.linux.dev" <linux-staging@...ts.linux.dev>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "xen-devel@...ts.xenproject.org" <xen-devel@...ts.xenproject.org>,
        "Christoph Hellwig" <hch@....de>
Subject: RE: [PATCH v3 3/4] vsscanf(): do not skip spaces

From: Demi Marie Obenour
> Sent: 10 June 2023 21:41
> 
> Passing spaces before e.g. an integer is usually
> not intended.  This was suggested by Christoph in
> https://lore.kernel.org/lkml/ZIQrohcizoj4bZWx@infradead.org/.

This is contrary to libc scanf and could easily affect userspace
writing fixed width values into sysctl nodes (etc).

IIRC strtoul() (etc) are also expected to strip leading spaces.
Removing the sign in sscanf() may lead to "-    12" being
valid - which may not be desired.

	David

> 
> Suggested-by: Christoph Hellwig <hch@....de>
> Signed-off-by: Demi Marie Obenour <demi@...isiblethingslab.com>
> ---
>  lib/vsprintf.c | 8 +-------
>  1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 9e53355c35b1d6260631868228ede1d178fe3325..665f6197f8313d653f67d7886b12c43942e058dd 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -3551,8 +3551,6 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
>  			char *s = (char *)va_arg(args, char *);
>  			if (field_width == -1)
>  				field_width = SHRT_MAX;
> -			/* first, skip leading white space in buffer */
> -			str = skip_spaces(str);
> 
>  			/* now copy until next white space */
>  			while (*str && !isspace(*str) && field_width--)
> @@ -3639,11 +3637,7 @@ int vsscanf(const char *buf, const char *fmt, va_list args)
>  			return num;
>  		}
> 
> -		/* have some sort of integer conversion.
> -		 * first, skip white space in buffer.
> -		 */
> -		str = skip_spaces(str);
> -
> +		/* have some sort of integer conversion. */
>  		digit = *str;
>  		if (is_sign && digit == '-') {
>  			if (field_width == 1)
> --
> Sincerely,
> Demi Marie Obenour (she/her/hers)
> Invisible Things Lab

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