krb5 commit: Zero length fields when freeing object contents

https://github.com/krb5/krb5/commit/4a2c5d259f5a7eda0f0f9028c061fcd032a72de0
commit 4a2c5d259f5a7eda0f0f9028c061fcd032a72de0
Author: Isaac Boukris <[email protected]>
Date:   Sun Jan 26 21:49:47 2020 +0100

    Zero length fields when freeing object contents
    
    In krb5_free_data_contents() and krb5_free_checksum_contents(), zero
    the length as well as the data pointer to leave the object in a valid
    state.  Add asserts to existing test harnesses to verify the new
    behavior.
    
    In the krb5 GSS mech's kg_checksum_channel_bindings(), remove the code
    to reallocate the checksum with xmalloc(), as it relied on
    krb5_free_checksum_contents() leaving the object in an invalid state.
    This code was added in commit a30fb4c4400f13a2690df7ef910b7ac0ccbcf194
    to match an xfree() call, but commit
    29337e7c7b796685fb6a03466d32147e17aa2d16 replaced that xfree() with a
    krb5_free_checksum_contents().  (In addition, the xmalloc and xfree
    wrappers never evolved to do anything beyond malloc and free.)
    
    In kpropd's recv_database(), don't free outbuf until we are done using
    its length.
    
    [[email protected]: rewrote commit message; edited doxygen comment
    changes to mention version]
    
    ticket: 8871 (new)

 src/include/krb5/krb5.hin              |    4 ++++
 src/kprop/kpropd.c                     |    2 +-
 src/lib/crypto/crypto_tests/t_cksums.c |    1 +
 src/lib/gssapi/krb5/util_cksum.c       |   16 ----------------
 src/lib/krb5/krb/kfree.c               |    8 ++++----
 src/tests/rdreq.c                      |    2 ++
 6 files changed, 12 insertions(+), 21 deletions(-)

diff --git a/src/include/krb5/krb5.hin b/src/include/krb5/krb5.hin
index d486853..4cd9ad5 100644
--- a/src/include/krb5/krb5.hin
+++ b/src/include/krb5/krb5.hin
@@ -4692,6 +4692,8 @@ krb5_free_checksum(krb5_context context, krb5_checksum *val);
  * @param [in] val              Checksum structure to free contents of
  *
  * This function frees the contents of @a val, but not the structure itself.
+ * It sets the checksum's data pointer to null and (beginning in release 1.19)
+ * sets its length to zero.
  */
 void KRB5_CALLCONV
 krb5_free_checksum_contents(krb5_context context, krb5_checksum *val);
@@ -4751,6 +4753,8 @@ krb5_free_octet_data(krb5_context context, krb5_octet_data *val);
  * @param [in] val              Data structure to free contents of
  *
  * This function frees the contents of @a val, but not the structure itself.
+ * It sets the structure's data pointer to null and (beginning in release 1.19)
+ * sets its length to zero.
  */
 void KRB5_CALLCONV
 krb5_free_data_contents(krb5_context context, krb5_data *val);
diff --git a/src/kprop/kpropd.c b/src/kprop/kpropd.c
index 5622d56..ab4a764 100644
--- a/src/kprop/kpropd.c
+++ b/src/kprop/kpropd.c
@@ -1412,7 +1412,6 @@ recv_database(krb5_context context, int fd, int database_fd,
         }
         n = write(database_fd, outbuf.data, outbuf.length);
         krb5_free_data_contents(context, &inbuf);
-        krb5_free_data_contents(context, &outbuf);
         if (n < 0) {
             snprintf(buf, sizeof(buf),
                      "while writing database block starting at offset %d",
@@ -1426,6 +1425,7 @@ recv_database(krb5_context context, int fd, int database_fd,
             send_error(context, fd, KRB5KRB_ERR_GENERIC, buf);
         }
         received_size += outbuf.length;
+        krb5_free_data_contents(context, &outbuf);
     }
 
     /* OK, we've seen the entire file.  Did we get too many bytes? */
diff --git a/src/lib/crypto/crypto_tests/t_cksums.c b/src/lib/crypto/crypto_tests/t_cksums.c
index 4da14ea..8297fcb 100644
--- a/src/lib/crypto/crypto_tests/t_cksums.c
+++ b/src/lib/crypto/crypto_tests/t_cksums.c
@@ -254,6 +254,7 @@ main(int argc, char **argv)
         }
 
         krb5_free_checksum_contents(context, &cksum);
+        assert(cksum.length == 0);
     }
     return status;
 }
diff --git a/src/lib/gssapi/krb5/util_cksum.c b/src/lib/gssapi/krb5/util_cksum.c
index a177077..2d6b50b 100644
--- a/src/lib/gssapi/krb5/util_cksum.c
+++ b/src/lib/gssapi/krb5/util_cksum.c
@@ -39,7 +39,6 @@ kg_checksum_channel_bindings(context, cb, cksum)
     size_t sumlen;
     krb5_data plaind;
     krb5_error_code code;
-    void *temp;
 
     /* initialize the the cksum */
     code = krb5_c_checksum_length(context, CKSUMTYPE_RSA_MD5, &sumlen);
@@ -88,21 +87,6 @@ kg_checksum_channel_bindings(context, cb, cksum)
 
     code = krb5_c_make_checksum(context, CKSUMTYPE_RSA_MD5, 0, 0,
                                 &plaind, cksum);
-    if (code)
-        goto cleanup;
-
-    if ((temp = xmalloc(cksum->length)) == NULL) {
-        krb5_free_checksum_contents(context, cksum);
-        code = ENOMEM;
-        goto cleanup;
-    }
-
-    memcpy(temp, cksum->contents, cksum->length);
-    krb5_free_checksum_contents(context, cksum);
-    cksum->contents = (krb5_octet *)temp;
-
-    /* success */
-cleanup:
     if (buf)
         xfree(buf);
     return code;
diff --git a/src/lib/krb5/krb/kfree.c b/src/lib/krb5/krb/kfree.c
index ab2409f..6e38044 100644
--- a/src/lib/krb5/krb/kfree.c
+++ b/src/lib/krb5/krb/kfree.c
@@ -145,6 +145,7 @@ krb5_free_checksum_contents(krb5_context context, krb5_checksum *val)
         return;
     free(val->contents);
     val->contents = NULL;
+    val->length = 0;
 }
 
 void KRB5_CALLCONV
@@ -242,10 +243,9 @@ krb5_free_data_contents(krb5_context context, krb5_data *val)
 {
     if (val == NULL)
         return;
-    if (val->data) {
-        free(val->data);
-        val->data = 0;
-    }
+    free(val->data);
+    val->data = NULL;
+    val->length = 0;
 }
 
 void KRB5_CALLCONV
diff --git a/src/tests/rdreq.c b/src/tests/rdreq.c
index c010cb2..52bec18 100644
--- a/src/tests/rdreq.c
+++ b/src/tests/rdreq.c
@@ -33,6 +33,7 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
+#include <assert.h>
 #include <krb5.h>
 
 int
@@ -105,6 +106,7 @@ main(int argc, char **argv)
     }
 
     krb5_free_data_contents(context, &apreq);
+    assert(apreq.length == 0);
     krb5_auth_con_free(context, auth_con);
     krb5_free_creds(context, cred);
     krb5_cc_close(context, ccache);