> Index: sbin/init/init.c
> ===================================================================
> RCS file: /cvs/src/sbin/init/init.c,v
> retrieving revision 1.63
> diff -u -p -u -r1.63 init.c
> --- sbin/init/init.c  2 Mar 2017 10:38:09 -0000       1.63
> +++ sbin/init/init.c  4 Apr 2017 08:50:53 -0000
> @@ -561,12 +561,13 @@ f_single_user(void)
>                       write(STDERR_FILENO, banner, sizeof banner - 1);
>                       for (;;) {
>                               int ok = 0;
> -                             clear = readpassphrase("Password:", pbuf, 
> sizeof(pbuf), RPP_ECHO_OFF);
> +                             clear = readpassphrase("Password:", pbuf,
> +                                 sizeof(pbuf), RPP_ECHO_OFF);
>                               if (clear == NULL || *clear == '\0')
>                                       _exit(0);
>                               if (crypt_checkpass(clear, pp->pw_passwd) == 0)
>                                       ok = 1;
> -                             memset(clear, 0, strlen(clear));
> +                             explicit_bzero(clear, strlen(clear));
>                               if (ok)
>                                       break;
>                               warning("single-user login failed\n");


I wonder whether explicit_bzero of pbuf would be better.

> Index: usr.bin/encrypt/encrypt.c
> ===================================================================
> RCS file: /cvs/src/usr.bin/encrypt/encrypt.c,v
> retrieving revision 1.45
> diff -u -p -u -r1.45 encrypt.c
> --- usr.bin/encrypt/encrypt.c 4 Sep 2016 15:36:13 -0000       1.45
> +++ usr.bin/encrypt/encrypt.c 4 Apr 2017 08:51:00 -0000
> @@ -134,6 +134,7 @@ main(int argc, char **argv)
>                               err(1, "readpassphrase");
>                       print_passwd(string, operation, extra);
>                       (void)fputc('\n', stdout);
> +                     explicit_bzero(string, sizeof(string));
>               } else {
>                       size_t len;
>                       /* Encrypt stdin to stdout. */

It is heading straight return (0) from main().  On the edge of useless,
because if you have a bug in logic that simple..


Let me stop here and ask if the pattern is: "always explicit_bzero
a password field once it is used"?  It might make sense, but some
of these are heading straight to exit immediately.  Is it too much
to do it then, or is the worry these code patterns might get copied
elsewhere?

Reply via email to