Deprecated calls to bzero() and index() found in OpenSSH 6.1p1

Bill Parker wp02855 at gmail.com
Fri Dec 21 06:03:01 EST 2012


Hello All,

    In reviewing source code for OpenSSH-6.1p1, I found instances
of deprecated library calls still within various source code files.

Examples of deprecated calls are: bzero() (replaced with memset()
which is ANSI compliant), index() (replaced with strchr() which
is also ANSI compliant).

In file 'auth2-jpake.c', I've replaced all the bzero() calls with
the equivalent memset() calls.  The patch file is below in (diff -u)
format:

--- auth2-jpake.c.orig  2012-12-19 17:01:52.817528171 -0800
+++ auth2-jpake.c       2012-12-19 17:05:59.044554766 -0800
@@ -178,7 +178,7 @@
                fatal("%s: not enough bytes for rawsalt (want %u have %u)",
                    __func__, len, digest_len);
        memcpy(rawsalt, digest, len);
-       bzero(digest, digest_len);
+        memset(digest, 0, digest_len);
        xfree(digest);
 }

@@ -203,10 +203,10 @@
                fatal("%s: want %u", __func__, want);

        derive_rawsalt(user, rawsalt, sizeof(rawsalt));
-       bzero(ret, sizeof(ret));
+        memset(ret, 0, sizeof(ret));
        for (i = 0; i < want; i++)
                ret[i] = pw_encode64(rawsalt[i]);
-       bzero(rawsalt, sizeof(rawsalt));
+        memset(rawsalt, 0, sizeof(rawsalt));

        return ret;
 }
@@ -360,7 +360,7 @@
        debug3("%s: scheme = %s", __func__, *hash_scheme);
        JPAKE_DEBUG_BN((*s, "%s: s = ", __func__));
 #endif
-       bzero(secret, secret_len);
+        memset(secret, 0, secret_len);
        xfree(secret);
 }

@@ -401,12 +401,12 @@
        packet_send();
        packet_write_wait();

-       bzero(hash_scheme, strlen(hash_scheme));
-       bzero(salt, strlen(salt));
+        memset(hash_scheme, 0, strlen(hash_scheme));
+        memset(salt, 0, strlen(salt));
        xfree(hash_scheme);
        xfree(salt);
-       bzero(x3_proof, x3_proof_len);
-       bzero(x4_proof, x4_proof_len);
+        memset(x3_proof, 0, x3_proof_len);
+        memset(x4_proof, 0, x4_proof_len);
        xfree(x3_proof);
        xfree(x4_proof);

@@ -453,8 +453,8 @@
            &pctx->b,
            &x4_s_proof, &x4_s_proof_len));

-       bzero(x1_proof, x1_proof_len);
-       bzero(x2_proof, x2_proof_len);
+        memset(x1_proof, 0, x1_proof_len);
+        mmeset(x2_proof, 0, x2_proof_len);
        xfree(x1_proof);
        xfree(x2_proof);

@@ -468,7 +468,7 @@
        packet_send();
        packet_write_wait();

-       bzero(x4_s_proof, x4_s_proof_len);
+        memset(x4_s_proof, 0, x4_s_proof_len);
        xfree(x4_s_proof);

        /* Expect step 2 packet from peer */
@@ -509,7 +509,7 @@
            &pctx->k,
            &pctx->h_k_sid_sessid, &pctx->h_k_sid_sessid_len));

-       bzero(x2_s_proof, x2_s_proof_len);
+        memset(x2_s_proof, 0, x2_s_proof_len);
        xfree(x2_s_proof);

        if (!use_privsep)
In file 'authfd.c', I've replaced the bzero() call with
the equivalent memset() call.  The patch file is below in
(diff -u) format:

--- authfd.c.orig       2012-12-19 17:07:08.292534290 -0800
+++ authfd.c    2012-12-19 17:10:48.240556291 -0800
@@ -102,7 +102,7 @@
        if (!authsocket)
                return -1;

-       bzero(&sunaddr, sizeof(sunaddr));
+        memset(&sunaddr, 0, sizeof(sunaddr));
        sunaddr.sun_family = AF_UNIX;
        strlcpy(sunaddr.sun_path, authsocket, sizeof(sunaddr.sun_path));
In file 'authfile.c', I've replaced the bzero() calls with
the equivalent memset() calls.  The patch file is below in
(diff -u) format:

--- authfile.c.orig     2012-12-19 17:11:06.537523347 -0800
+++ authfile.c  2012-12-19 17:12:03.999868720 -0800
@@ -349,17 +349,17 @@
                            __func__, filename == NULL ? "" : filename,
                            filename == NULL ? "" : " ", strerror(errno));
                        buffer_clear(blob);
-                       bzero(buf, sizeof(buf));
+                        memset(buf, 0, sizeof(buf));
                        return 0;
                }
                buffer_append(blob, buf, len);
                if (buffer_len(blob) > MAX_KEY_FILE_SIZE) {
                        buffer_clear(blob);
-                       bzero(buf, sizeof(buf));
+                        memset(buf, 0, sizeof(buf));
                        goto toobig;
                }
        }
-       bzero(buf, sizeof(buf));
+        memset(buf, 0, sizeof(buf));
        if ((st.st_mode & (S_IFSOCK|S_IFCHR|S_IFIFO)) == 0 &&
            st.st_size != buffer_len(blob)) {
                debug("%s: key file %.200s%schanged size while reading",
In file 'bufaux.c', I've replaced the bzero() call with
the equivalent memset() call.  The patch file is below in
(diff -u) format:

--- bufaux.c.orig       2012-12-19 17:12:21.672509848 -0800
+++ bufaux.c    2012-12-19 17:12:44.902553992 -0800
@@ -215,7 +215,7 @@
                if (cp == ret + length - 1)
                        error("buffer_get_cstring_ret: string contains
\\0");
                else {
-                       bzero(ret, length);
+                        memset(ret, 0, length);
                        xfree(ret);
                        return NULL;
                }
In file 'bufec.c', I've replaced the bzero() calls with
the equivalent memset() calls.  The patch file is below in
(diff -u) format:

--- bufec.c.orig        2012-12-19 17:13:00.367528026 -0800
+++ bufec.c     2012-12-19 17:13:26.725554444 -0800
@@ -77,7 +77,7 @@
        ret = 0;
  out:
        if (buf != NULL) {
-               bzero(buf, len);
+                memset(buf, 0, len);
                xfree(buf);
        }
        BN_CTX_free(bnctx);
@@ -130,7 +130,7 @@
        ret = 0;
  out:
        BN_CTX_free(bnctx);
-       bzero(buf, len);
+        memset(buf, 0, len);
        xfree(buf);
        return ret;
 }

In file 'canohost.c', I've replaced the bzero() call with
the equivalent memset() call.  The patch file is below in
(diff -u) format:

--- canohost.c.orig     2012-12-19 17:13:41.958551344 -0800
+++ canohost.c  2012-12-19 17:14:05.773567216 -0800
@@ -199,7 +199,7 @@
        memcpy(&inaddr, ((char *)&a6->sin6_addr) + 12, sizeof(inaddr));
        port = a6->sin6_port;

-       bzero(a4, sizeof(*a4));
+        memset(a4, 0, sizeof(*a4));

        a4->sin_family = AF_INET;
        *len = sizeof(*a4);
In file 'channels.c', I've replaced the bzero() calls with
the equivalent memset() calls.  The patch file is below in
(diff -u) format:

--- channels.c.orig     2012-12-19 17:14:18.911938266 -0800
+++ channels.c  2012-12-19 17:15:04.724553211 -0800
@@ -425,7 +425,7 @@
                if (cc->abandon_cb != NULL)
                        cc->abandon_cb(c, cc->ctx);
                TAILQ_REMOVE(&c->status_confirms, cc, entry);
-               bzero(cc, sizeof(*cc));
+                memset(cc, 0, sizeof(*cc));
                xfree(cc);
        }
        if (c->filter_cleanup != NULL && c->filter_ctx != NULL)
@@ -2667,7 +2667,7 @@
                return;
        cc->cb(type, c, cc->ctx);
        TAILQ_REMOVE(&c->status_confirms, cc, entry);
-       bzero(cc, sizeof(*cc));
+        memset(cc, 0, sizeof(*cc));
        xfree(cc);
 }

@@ -3296,7 +3296,7 @@
        xfree(cctx->host);
        if (cctx->aitop)
                freeaddrinfo(cctx->aitop);
-       bzero(cctx, sizeof(*cctx));
+        memset(cctx, 0, sizeof(*cctx));
        cctx->host = NULL;
        cctx->ai = cctx->aitop = NULL;
 }

In file 'clientloop.c', I've replaced the bzero() calls with
the equivalent memset() calls.  The patch file is below in
(diff -u) format:

--- clientloop.c.orig   2012-12-19 17:15:16.548673571 -0800
+++ clientloop.c        2012-12-19 17:15:52.244510839 -0800
@@ -551,7 +551,7 @@
                gc->cb(type, seq, gc->ctx);
        if (--gc->ref_count <= 0) {
                TAILQ_REMOVE(&global_confirms, gc, entry);
-               bzero(gc, sizeof(*gc));
+                memset(gc, 0, sizeof(*gc));
                xfree(gc);
        }

@@ -867,7 +867,7 @@
        int cancel_port, ok;
        Forward fwd;

-       bzero(&fwd, sizeof(fwd));
+        memset(&fwd, 0, sizeof(fwd));
        fwd.listen_host = fwd.connect_host = NULL;

        leave_raw_mode(options.request_tty == REQUEST_TTY_FORCE);
In file 'hostfile.c', I've replaced the bzero() call with
the equivalent memset() call.  The patch file is below in
(diff -u) format:

--- hostfile.c.orig     2012-12-19 17:19:15.393667218 -0800
+++ hostfile.c  2012-12-19 17:19:50.752556571 -0800
@@ -330,7 +330,7 @@
                xfree(hostkeys->entries[i].host);
                xfree(hostkeys->entries[i].file);
                key_free(hostkeys->entries[i].key);
-               bzero(hostkeys->entries + i, sizeof(*hostkeys->entries));
+                memset(hostkeys->entries + i, 0,
sizeof(*hostkeys->entries));
        }
        if (hostkeys->entries != NULL)
                xfree(hostkeys->entries);
In file 'jpake.c', I've replaced the bzero() calls with
the equivalent memset() calls.  The patch file is below in
(diff -u) format:

--- jpake.c.orig        2012-12-19 17:20:06.946529817 -0800
+++ jpake.c     2012-12-19 17:21:32.343510968 -0800
@@ -105,7 +105,7 @@
 #define JPAKE_BUF_CLEAR_FREE(v, l)             \
        do {                                    \
                if ((v) != NULL) {              \
-                       bzero((v), (l));        \
+                        memset((v), 0, (l));    \
                        xfree(v);               \
                        (v) = NULL;             \
                        (l) = 0;                \
@@ -133,7 +133,7 @@
 #undef JPAKE_BN_CLEAR_FREE
 #undef JPAKE_BUF_CLEAR_FREE

-       bzero(pctx, sizeof(*pctx));
+        memset(pctx, 0, sizeof(*pctx));
        xfree(pctx);
 }

@@ -444,7 +444,7 @@
        else if (timingsafe_bcmp(peer_confirm_hash, expected_confirm_hash,
            expected_confirm_hash_len) == 0)
                success = 1;
-       bzero(expected_confirm_hash, expected_confirm_hash_len);
+        memset(expected_confirm_hash, 0, expected_confirm_hash_len);
        xfree(expected_confirm_hash);
        debug3("%s: success = %d", __func__, success);
        return success;
In file 'monitor.c', I've replaced the bzero() calls with
the equivalent memset() calls.  The patch file is below in
(diff -u) format:

--- monitor.c.orig      2012-12-19 17:22:45.966559767 -0800
+++ monitor.c   2012-12-19 17:26:41.827534855 -0800
@@ -547,7 +547,7 @@
        struct pollfd pfd[2];

        for (;;) {
-               bzero(&pfd, sizeof(pfd));
+                memset(&pfd, 0, sizeof(pfd));
                pfd[0].fd = pmonitor->m_sendfd;
                pfd[0].events = POLLIN;
                pfd[1].fd = pmonitor->m_log_recvfd;
@@ -2137,8 +2137,8 @@
        debug3("%s: sending step1", __func__);
        mm_request_send(sock, MONITOR_ANS_JPAKE_STEP1, m);

-       bzero(x3_proof, x3_proof_len);
-       bzero(x4_proof, x4_proof_len);
+        memset(x3_proof, 0, x3_proof_len);
+        memset(x4_proof, 0, x4_proof_len);
        xfree(x3_proof);
        xfree(x4_proof);

@@ -2167,8 +2167,8 @@
        debug3("%s: sending pwdata", __func__);
        mm_request_send(sock, MONITOR_ANS_JPAKE_GET_PWDATA, m);

-       bzero(hash_scheme, strlen(hash_scheme));
-       bzero(salt, strlen(salt));
+        memset(hash_scheme, 0, strlen(hash_scheme));
+        memset(salt, 0, strlen(salt));
        xfree(hash_scheme);
        xfree(salt);

@@ -2207,8 +2207,8 @@

        JPAKE_DEBUG_CTX((pctx, "step2 done in %s", __func__));

-       bzero(x1_proof, x1_proof_len);
-       bzero(x2_proof, x2_proof_len);
+        memset(x1_proof, 0, x1_proof_len);
+        memset(x2_proof, 0, x2_proof_len);
        xfree(x1_proof);
        xfree(x2_proof);

@@ -2220,7 +2220,7 @@
        debug3("%s: sending step2", __func__);
        mm_request_send(sock, MONITOR_ANS_JPAKE_STEP2, m);

-       bzero(x4_s_proof, x4_s_proof_len);
+        memset(x4_s_proof, 0, x4_s_proof_len);
        xfree(x4_s_proof);

        monitor_permit(mon_dispatch, MONITOR_REQ_JPAKE_KEY_CONFIRM, 1);
@@ -2254,7 +2254,7 @@

        JPAKE_DEBUG_CTX((pctx, "key_confirm done in %s", __func__));

-       bzero(x2_s_proof, x2_s_proof_len);
+        memset(x2_s_proof, 0, x2_s_proof_len);
        buffer_clear(m);

        /* pctx->k is sensitive, not sent */
@@ -2288,7 +2288,7 @@

        JPAKE_DEBUG_CTX((pctx, "check_confirm done in %s", __func__));

-       bzero(peer_confirm_hash, peer_confirm_hash_len);
+        memset(peer_confirm_hash, 0, peer_confirm_hash_len);
        xfree(peer_confirm_hash);

        buffer_clear(m);
In file 'sandbox-systrace.c', I've replaced the bzero() call with
the equivalent memset() call.  The patch file is below in
(diff -u) format:

--- sandbox-systrace.c.orig     2012-12-19 17:27:48.258532654 -0800
+++ sandbox-systrace.c  2012-12-19 17:28:12.705825672 -0800
@@ -140,7 +140,7 @@
                    box->systrace_fd, child_pid, strerror(errno));

        /* Allocate and assign policy */
-       bzero(&policy, sizeof(policy));
+        memset(&policy, 0, sizeof(policy));
        policy.strp_op = SYSTR_POLICY_NEW;
        policy.strp_maxents = SYS_MAXSYSCALL;
        if (ioctl(box->systrace_fd, STRIOCPOLICY, &policy) == -1)
In file 'schnorr.c', I've replaced the bzero() calls with
the equivalent memset() calls.  The patch file is below in
(diff -u) format:

--- schnorr.c.orig      2012-12-19 17:28:24.666675070 -0800
+++ schnorr.c   2012-12-19 17:29:14.920563605 -0800
@@ -101,7 +101,7 @@
        SCHNORR_DEBUG_BN((h, "%s: h = ", __func__));
  out:
        buffer_free(&b);
-       bzero(digest, digest_len);
+        memset(digest, 0, digest_len);
        xfree(digest);
        digest_len = 0;
        if (success == 0)
@@ -477,7 +477,7 @@
        success = 0;
  out:
        EVP_MD_CTX_cleanup(&evp_md_ctx);
-       bzero(digest, sizeof(digest));
+        memset(digest, 0, sizeof(digest));
        digest_len = 0;
        return success;
 }
@@ -570,7 +570,7 @@
                BN_clear_free(grp->p);
        if (grp->q != NULL)
                BN_clear_free(grp->q);
-       bzero(grp, sizeof(*grp));
+        memset(grp, 0, sizeof(*grp));
        xfree(grp);
 }

In file 'session.c', I've replaced the bzero() call with
the equivalent memset() call.  The patch file is below in
(diff -u) format:

--- session.c.orig      2012-12-19 17:29:24.289506673 -0800
+++ session.c   2012-12-19 17:29:50.967542588 -0800
@@ -1840,7 +1840,7 @@
                fatal("%s: insane session id %d (max %d nalloc %d)",
                    __func__, id, options.max_sessions, sessions_nalloc);
        }
-       bzero(&sessions[id], sizeof(*sessions));
+        memset(&sessions[id], 0, sizeof(*sessions));
        sessions[id].self = id;
        sessions[id].used = 0;
        sessions[id].chanid = -1;
In file 'sftp-client.c', I've replaced the bzero() call with
the equivalent memset() call.  The patch file is below in
(diff -u) format:

--- sftp-client.c.orig  2012-12-19 17:30:09.345533585 -0800
+++ sftp-client.c       2012-12-19 17:30:34.055766403 -0800
@@ -308,7 +308,7 @@
                    SSH2_FXP_EXTENDED_REPLY, type);
        }

-       bzero(st, sizeof(*st));
+        memset(st, 0, sizeof(*st));
        st->f_bsize = buffer_get_int64(&msg);
        st->f_frsize = buffer_get_int64(&msg);
        st->f_blocks = buffer_get_int64(&msg);
In file 'sshconnect2.c', I've replaced the bzero() calls with
the equivalent memset() calls.  The patch file is below in
(diff -u) format:

--- sshconnect2.c.orig  2012-12-19 17:32:38.389551253 -0800
+++ sshconnect2.c       2012-12-19 17:35:55.170602684 -0800
@@ -1010,14 +1010,14 @@
            &secret, &secret_len) != 0)
                fatal("%s: hash_buffer", __func__);

-       bzero(password, strlen(password));
-       bzero(crypted, strlen(crypted));
+        memset(password, 0, strlen(password));
+        memset(crypted, 0, strlen(crypted));
        xfree(password);
        xfree(crypted);

        if ((ret = BN_bin2bn(secret, secret_len, NULL)) == NULL)
                fatal("%s: BN_bin2bn (secret)", __func__);
-       bzero(secret, secret_len);
+        memset(secret, 0, secret_len);
        xfree(secret);

        return ret;
@@ -1054,8 +1054,8 @@

        /* Obtain password and derive secret */
        pctx->s = jpake_password_to_secret(authctxt, crypt_scheme, salt);
-       bzero(crypt_scheme, strlen(crypt_scheme));
-       bzero(salt, strlen(salt));
+        memset(crypt_scheme, 0, strlen(crypt_scheme));
+        memset(salt, 0, strlen(salt));
        xfree(crypt_scheme);
        xfree(salt);
        JPAKE_DEBUG_BN((pctx->s, "%s: s = ", __func__));
@@ -1070,8 +1070,8 @@
            &pctx->a,
            &x2_s_proof, &x2_s_proof_len);

-       bzero(x3_proof, x3_proof_len);
-       bzero(x4_proof, x4_proof_len);
+        memset(x3_proof, 0, x3_proof_len);
+        memset(x4_proof, 0, x4_proof_len);
        xfree(x3_proof);
        xfree(x4_proof);

@@ -1083,7 +1083,7 @@
        packet_put_string(x2_s_proof, x2_s_proof_len);
        packet_send();

-       bzero(x2_s_proof, x2_s_proof_len);
+        memset(x2_s_proof, 0, x2_s_proof_len);
        xfree(x2_s_proof);

        /* Expect step 2 packet from peer */
@@ -1123,7 +1123,7 @@
            &pctx->k,
            &pctx->h_k_cid_sessid, &pctx->h_k_cid_sessid_len);

-       bzero(x4_s_proof, x4_s_proof_len);
+        memset(x4_s_proof, 0, x4_s_proof_len);
        xfree(x4_s_proof);

        JPAKE_DEBUG_CTX((pctx, "confirm sending in %s", __func__));
@@ -1787,8 +1787,8 @@
        packet_put_string(x2_proof, x2_proof_len);
        packet_send();

-       bzero(x1_proof, x1_proof_len);
-       bzero(x2_proof, x2_proof_len);
+        memset(x1_proof, 0, x1_proof_len);
+        memset(x2_proof, 0, x2_proof_len);
        xfree(x1_proof);
        xfree(x2_proof);

In file 'ssh.c', I've replaced the bzero() calls with
the equivalent memset() calls.  The patch file is below in
(diff -u) format:

--- ssh.c.orig  2012-12-19 17:30:53.992528775 -0800
+++ ssh.c       2012-12-19 17:32:16.421511581 -0800
@@ -1509,8 +1509,8 @@
 #endif /* PKCS11 */

        n_ids = 0;
-       bzero(identity_files, sizeof(identity_files));
-       bzero(identity_keys, sizeof(identity_keys));
+        memset(identity_files, 0, sizeof(identity_files));
+        memset(identity_keys, 0, sizeof(identity_keys));

 #ifdef ENABLE_PKCS11
        if (options.pkcs11_provider != NULL &&
@@ -1584,9 +1584,9 @@
        memcpy(options.identity_files, identity_files,
sizeof(identity_files));
        memcpy(options.identity_keys, identity_keys, sizeof(identity_keys));

-       bzero(pwname, strlen(pwname));
+        memset(pwname, 0, strlen(pwname));
        xfree(pwname);
-       bzero(pwdir, strlen(pwdir));
+        memset(pwdir, 0, strlen(pwdir));
        xfree(pwdir);
 }

In file 'ssh-keygen.c', I've replaced the bzero() call with
the equivalent memset() call.  The patch file is below in
(diff -u) format:

--- ssh-keygen.c.orig   2012-12-19 17:21:53.056678875 -0800
+++ ssh-keygen.c        2012-12-19 17:22:21.022556542 -0800
@@ -1660,7 +1660,7 @@
                fatal("Invalid certificate time format %s", s);
        }

-       bzero(&tm, sizeof(tm));
+        memset(&tm, 0, sizeof(tm));
        if (strptime(buf, fmt, &tm) == NULL)
                fatal("Invalid certificate time %s", s);
        if ((tt = mktime(&tm)) < 0)
In directory 'openbsd-compat', file 'ssh-keygen.c', I've
replaced the index() call with the equivalent strchr() call.
The patch file is below in (diff -u) format:

--- port-linux.c.orig   2012-12-19 17:40:53.231529475 -0800
+++ port-linux.c        2012-12-19 17:41:27.573571514 -0800
@@ -191,7 +191,7 @@
                logit("%s: getcon failed with %s", __func__,
strerror(errno));
                return;
        }
-       if ((cx = index(oldctx, ':')) == NULL || (cx = index(cx + 1, ':'))
==
+       if ((cx = strchr(oldctx, ':')) == NULL || (cx = strchr(cx + 1,
':')) ==
            NULL) {
                logit ("%s: unparseable context %s", __func__, oldctx);
                return;
@@ -210,7 +210,7 @@
        len = cx - oldctx + 1;
        memcpy(newctx, oldctx, len);
        strlcpy(newctx + len, newname, newlen - len);
-       if ((cx = index(cx + 1, ':')))
+       if ((cx = strchr(cx + 1, ':')))
                strlcat(newctx, cx, newlen);
        debug3("%s: setting context from '%s' to '%s'", __func__,
            oldctx, newctx);
A configure with the following parameters below:

./configure --with-pam --with-md5-passwords --with-kerberos5=/usr/kerberos
--with-tcp-wrappers

and 'make' result in clean configure and make.

I am attaching all of the patch files (in diff -u format) to
this email/bug report.  Please feel free to make any needed
changes, etc as needed.

Bill Parker (wp02855 at gmail dot com)


More information about the openssh-unix-dev mailing list