svn commit: r336619 - head/lib/libc/gen

[Cy Schubert]

In message <201807222234.w6MMYKpn030237 at repo.freebsd.org>, Ian Lepore 
writes:
> Author: ian
> Date: Sun Jul 22 22:34:20 2018
> New Revision: 336619
> URL: https://svnweb.freebsd.org/changeset/base/336619
>
> Log:
>   Set the pw_class field to NULL when scanning the non-master passwd file.
>   This avoids a null pointer deref in pw_dup(), which assumes that all
>   pointers are either NULL or valid.
>
> Modified:
>   head/lib/libc/gen/pw_scan.c
>
> Modified: head/lib/libc/gen/pw_scan.c
> =============================================================================
> =
> --- head/lib/libc/gen/pw_scan.c	Sun Jul 22 21:39:27 2018	(r33661
> 8)
> +++ head/lib/libc/gen/pw_scan.c	Sun Jul 22 22:34:20 2018	(r33661
> 9)
> @@ -170,7 +170,8 @@ __pw_scan(char *bp, struct passwd *pw, int flags)
>  		if (p[0])
>  			pw->pw_fields |= _PWF_EXPIRE;
>  		pw->pw_expire = atol(p);
> -	}
> +	} else
> +		pw->pw_class = NULL;
>  	if (!(pw->pw_gecos = strsep(&bp, ":")))		/* gecos */
>  		goto fmt;
>  	if (pw->pw_gecos[0])
>

Hi Ian,

This causes ssh a bit of gas.

slippy$ gdb ssh
GNU gdb (GDB) 8.1 [GDB v8.1 for FreeBSD]
Copyright (C) 2018 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.
html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.  Type "show 
copying"
and "show warranty" for details.
This GDB was configured as "x86_64-portbld-freebsd12.0".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<http://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
<http://www.gnu.org/software/gdb/documentation/>.
For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ssh...Reading symbols from /usr/lib/debug//usr/bin/
ssh.debug...done.
done.
(gdb) set args bob id
(gdb) run
Starting program: /usr/bin/ssh bob id

Program received signal SIGSEGV, Segmentation fault.
strlen (str=0x0) at /opt/src/svn-current/lib/libc/string/strlen.c:101
101		va = (*lp - mask01);
(gdb) bt
#0  strlen (str=0x0) at /opt/src/svn-current/lib/libc/string/strlen.c:10
1
#1  0x000000002bf10462 in Fssh_xstrdup (str=0x0)
    at /opt/src/svn-current/crypto/openssh/xmalloc.c:98
#2  0x000000002bf0ce38 in Fssh_pwcopy (pw=0x2c5962d0)
    at /opt/src/svn-current/crypto/openssh/misc.c:302
#3  0x0000000000214391 in main (ac=3, av=0x7fffffffe648)
    at /opt/src/svn-current/crypto/openssh/ssh.c:633
(gdb) l
96		 * p and (p & ~LONGPTR_MASK) must be equally accessible since
97		 * they always fall in the same memory page, as long as page
98		 * boundaries is integral multiple of word size.
99		 */
100		lp = (const unsigned long *)((uintptr_t)str & ~LONGPTR_MASK);
101		va = (*lp - mask01);
102		vb = ((~*lp) & mask80);
103		lp++;
104		if (va & vb)
105			/* Check if we have \0 in the first part */
(gdb) p lp
$1 = (const unsigned long *) 0x0
(gdb) up
#1  0x000000002bf10462 in Fssh_xstrdup (str=0x0)
    at /opt/src/svn-current/crypto/openssh/xmalloc.c:98
98		len = strlen(str) + 1;
(gdb) l
93	xstrdup(const char *str)
94	{
95		size_t len;
96		char *cp;
97	
98		len = strlen(str) + 1;
99		cp = xmalloc(len);
100		strlcpy(cp, str, len);
101		return cp;
102	}
(gdb) p str
$2 = 0x0
(gdb) up 
#2  0x000000002bf0ce38 in Fssh_pwcopy (pw=0x2c5962d0)
    at /opt/src/svn-current/crypto/openssh/misc.c:302
302		copy->pw_class = xstrdup(pw->pw_class);
(gdb) l
297	#endif
298	#ifdef HAVE_STRUCT_PASSWD_PW_CHANGE
299		copy->pw_change = pw->pw_change;
300	#endif
301	#ifdef HAVE_STRUCT_PASSWD_PW_CLASS
302		copy->pw_class = xstrdup(pw->pw_class);
303	#endif
304		copy->pw_dir = xstrdup(pw->pw_dir);
305		copy->pw_shell = xstrdup(pw->pw_shell);
306		return copy;
(gdb) p pw->pw_class
$3 = 0x0
(gdb) 

(If anyone's wondering, I name my systems after dead pets.)


-- 
Cheers,
Cy Schubert <Cy.Schubert at cschubert.com>
FreeBSD UNIX:  <cy at FreeBSD.org>   Web:  http://www.FreeBSD.org

	The need of the many outweighs the greed of the few.