Avoid passing null pointers to memcpy/memcmp

By a strict reading of the C standard, memcpy and memcmp have
undefined behavior if their pointer arguments aren't valid object
pointers, even if the length argument is 0.  Compilers are becoming
more aggressive about breaking code with undefined behavior, so we
should try to avoid it when possible.

In a krb5_data object, we frequently use NULL as the data value when
the length is 0.  Accordingly, we should avoid copying from or
comparing the data field of a length-0 krb5_data object.  Add checks
to our wrapper functions (like data_eq and k5_memdup) and to code
which works with possibly-empty krb5_data objects.  In a few places,
use wrapper functions to simplify the code rather than adding checks.

3 files changed, 16 insertions, 9 deletions

diff --git a/src/lib/crypto/krb/encrypt.c b/src/lib/crypto/krb/encrypt.c
index f46fb13..d9d575a 100644
--- a/src/lib/crypto/krb/encrypt.c
+++ b/src/lib/crypto/krb/encrypt.c
@@ -60,7 +60,8 @@ krb5_k_encrypt(krb5_context context, krb5_key key,
     iov[1].flags = KRB5_CRYPTO_TYPE_DATA;
     iov[1].data = make_data(output->ciphertext.data + header_len,
                             input->length);
-    memcpy(iov[1].data.data, input->data, input->length);
+    if (input->length > 0)
+        memcpy(iov[1].data.data, input->data, input->length);
 
     iov[2].flags = KRB5_CRYPTO_TYPE_PADDING;
     iov[2].data = make_data(iov[1].data.data + input->length, padding_len);
diff --git a/src/lib/crypto/krb/s2k_des.c b/src/lib/crypto/krb/s2k_des.c
index 61b3c0f..31a613b 100644
--- a/src/lib/crypto/krb/s2k_des.c
+++ b/src/lib/crypto/krb/s2k_des.c
@@ -404,7 +404,8 @@ afs_s2k_oneblock(const krb5_data *data, const krb5_data *salt,
      */
 
     memset(password, 0, sizeof(password));
-    memcpy(password, salt->data, min(salt->length, 8));
+    if (salt->length > 0)
+        memcpy(password, salt->data, min(salt->length, 8));
     for (i = 0; i < 8; i++) {
         if (isupper(password[i]))
             password[i] = tolower(password[i]);
@@ -443,7 +444,8 @@ afs_s2k_multiblock(const krb5_data *data, const krb5_data *salt,
     if (!password)
         return ENOMEM;
 
-    memcpy(password, data->data, data->length);
+    if (data->length > 0)
+        memcpy(password, data->data, data->length);
     for (i = data->length, j = 0; j < salt->length; i++, j++) {
         password[i] = salt->data[j];
         if (isupper(password[i]))
@@ -513,8 +515,9 @@ des_s2k(const krb5_data *pw, const krb5_data *salt, unsigned char *key_out)
     copy = malloc(copylen);
     if (copy == NULL)
         return ENOMEM;
-    memcpy(copy, pw->data, pw->length);
-    if (salt)
+    if (pw->length > 0)
+        memcpy(copy, pw->data, pw->length);
+    if (salt != NULL && salt->length > 0)
         memcpy(copy + pw->length, salt->data, salt->length);
 
     memset(&temp, 0, sizeof(temp));
diff --git a/src/lib/crypto/krb/s2k_pbkdf2.c b/src/lib/crypto/krb/s2k_pbkdf2.c
index 2476865..4ada811 100644
--- a/src/lib/crypto/krb/s2k_pbkdf2.c
+++ b/src/lib/crypto/krb/s2k_pbkdf2.c
@@ -61,8 +61,9 @@ krb5int_dk_string_to_key(const struct krb5_keytypes *ktp,
 
     /* construct input string ( = string + salt), fold it, make_key it */
 
-    memcpy(concat, string->data, string->length);
-    if (salt)
+    if (string->length > 0)
+        memcpy(concat, string->data, string->length);
+    if (salt != NULL && salt->length > 0)
         memcpy(concat + string->length, salt->data, salt->length);
 
     krb5int_nfold(concatlen*8, concat, keybytes*8, foldstring);
@@ -146,9 +147,11 @@ pbkdf2_string_to_key(const struct krb5_keytypes *ktp, const krb5_data *string,
         if (err)
             return err;
 
-        memcpy(sandp.data, pepper->data, pepper->length);
+        if (pepper->length > 0)
+            memcpy(sandp.data, pepper->data, pepper->length);
         sandp.data[pepper->length] = '\0';
-        memcpy(&sandp.data[pepper->length + 1], salt->data, salt->length);
+        if (salt->length > 0)
+            memcpy(&sandp.data[pepper->length + 1], salt->data, salt->length);
 
         salt = &sandp;
     }