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 a30fb4c
to match an xfree() call, but commit
29337e7 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.

[ghudson@mit.edu: rewrote commit message; edited doxygen comment
changes to mention version]

ticket: 8871 (new)

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);

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? */

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;
 }

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;

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

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);