[Bug 2541] New: Add explicit_bzero() before free() in OpenSSH-7.1p2 for auth1.c/auth2.c/auth2-hostbased.c

[bugzilla-daemon at bugzilla.mindrot.org]

https://bugzilla.mindrot.org/show_bug.cgi?id=2541

            Bug ID: 2541
           Summary: Add explicit_bzero() before free() in OpenSSH-7.1p2
                    for auth1.c/auth2.c/auth2-hostbased.c
           Product: Portable OpenSSH
           Version: 7.1p1
          Hardware: All
                OS: All
            Status: NEW
          Severity: normal
          Priority: P5
         Component: ssh
          Assignee: unassigned-bugs at mindrot.org
          Reporter: wp02855 at gmail.com

Created attachment 2787
  --> https://bugzilla.mindrot.org/attachment.cgi?id=2787&action=edit
Patch file for this bug report

Hello All,

        In reviewing code in OpenSSH-7.1p2, there are some instances
where free()
is called in file 'auth1.c', in which the contents are not sanitized
before free() is called.  The patch file below should address these
instances:

--- auth1.c.orig        2016-02-13 18:13:35.016278903 -0800
+++ auth1.c     2016-02-13 18:18:43.853530529 -0800
@@ -207,6 +207,7 @@
        debug("sending challenge '%s'", challenge);
        packet_start(SSH_SMSG_AUTH_TIS_CHALLENGE);
        packet_put_cstring(challenge);
+       explicit_bzero(challenge, sizeof(*challenge));
        free(challenge);
        packet_send();
        packet_write_wait();
@@ -356,6 +357,7 @@
                /* Log before sending the reply */
                auth_log(authctxt, authenticated, 0,
get_authname(type), NULL);

+               explicit_bzero(client_user, sizeof(*client_user));
                free(client_user);
                client_user = NULL;

=======================================================================

In the case of variable 'client_user', calling free() and setting the
static char pointer to NULL does not explicitly scrub any of the
contents
past the first character, does it not?

        In reviewing code in openssh-7.1p2, there are some instances
where free()
is called in file 'auth2.c', in which the contents are not sanitized
before free() is called.  The patch file below should address these
instances:

--- auth2.c.orig        2016-02-13 18:43:48.284735999 -0800
+++ auth2.c     2016-02-13 18:50:11.694465404 -0800
@@ -125,6 +125,7 @@
        close(fd);

        if (n != len) {
+               explicit_bzero(banner, sizeof(*banner));
                free(banner);
                return (NULL);
        }
@@ -159,6 +160,7 @@
        userauth_send_banner(banner);

 done:
+       explicit_bzero(banner, sizeof(*banner));
        free(banner);
 }

@@ -204,6 +206,7 @@
                debug("bad service request %s", service);
                packet_disconnect("bad service request %s", service);
        }
+       explicit_bzero(service, sizeof(*service))
        free(service);
        return 0;
 }
@@ -282,8 +285,11 @@
        }
        userauth_finish(authctxt, authenticated, method, NULL);

+       explicit_bzero(service, sizeof(*service));
        free(service);
+       explicit_bzero(user, sizeof(*user));
        free(user);
+       explicit_bzero(method, sizeof(*method));
        free(method);
        return 0;
 }
@@ -373,6 +379,7 @@
                packet_put_char(partial);
                packet_send();
                packet_write_wait();
+               explicit_bzero(methods, sizeof(*methods));
                free(methods);
        }
 }
@@ -491,6 +498,7 @@
        }
        ret = 0;
  out:
+       explicit_bzero(omethods, sizeof(*omethods));
        free(omethods);
        return ret;
 }
@@ -581,6 +589,7 @@
        if (*p == ',')
                p++;
        *methods = xstrdup(p);
+       explicit_bzero(omethods, sizeof(*omethods));
        free(omethods);
        return 1;
 }

=======================================================================

        In reviewing code in openssh-7.1p2, there are some instances
where free()
is called in file 'auth2-hostbased.c', in which the contents are not
sanitized
before free() is called.  The patch file below should address these
instances:


--- auth2-hostbased.c.orig      2016-02-13 19:00:19.828756146 -0800
+++ auth2-hostbased.c   2016-02-13 19:04:31.173700796 -0800
@@ -147,10 +147,15 @@
        debug2("userauth_hostbased: authenticated %d", authenticated);
        if (key != NULL)
                key_free(key);
+       explicit_bzero(pkalg, sizeof(*pkalg));
        free(pkalg);
+       explicit_bzero(pkblob, sizeof(*pkblob));
        free(pkblob);
+       explicit_bzero(cuser, sizeof(*cuser));
        free(cuser);
+       explicit_bzero(chost, sizeof(*chost));
        free(chost);
+       explicit_bzero(sig, sizeof(*sig));
        free(sig);
        return authenticated;
 }
@@ -237,6 +242,7 @@
                        verbose("Accepted %s public key %s from %s@%s",
                            key_type(key), fp, cuser, lookup);
                }
+               explicit_bzero(fp, sizeof(*fp));
                free(fp);
        }

=======================================================================

I am attaching the patch file(s) to this bug report...

-- 
You are receiving this mail because:
You are watching the assignee of the bug.