Avoid undefined memcpy/memmove invocations

Where memcpy() or memmove() might be invoked with null input or output
pointers, add checks for non-zero length.  For brevity, add helpers
k5memcpy() and k5memmove() and use them in files that include
k5-int.h.

Most of the potential undefined invocations were reported by Evgeny
Shemyakin.

diff --git a/src/include/k5-int.h b/src/include/k5-int.h
index cfd2cc9393d92c850b3d95b6f4f3bfe6a6b97e77..56d9d1b205084d1de63b4e88c4069b0dba551986 100644 (file)
--- a/src/include/k5-int.h
+++ b/src/include/k5-int.h
@@ -2313,6 +2313,22 @@ k5memdup0(const void *in, size_t len, krb5_error_code *code)
     return ptr;
 }
 
+/* memcpy if len > 0, to avoid passing null pointers (which is undefined). */
+static inline void
+k5memcpy(void *dest, const void *src, size_t len)
+{
+    if (len > 0)
+        memcpy(dest, src, len);
+}
+
+/* memmove if len > 0, to avoid passing null pointers (which is undefined). */
+static inline void
+k5memmove(void *dest, const void *src, size_t len)
+{
+    if (len > 0)
+        memmove(dest, src, len);
+}
+
 /* Convert a krb5_timestamp to a time_t value, treating the negative range of
  * krb5_timestamp as times between 2038 and 2106 (if time_t is 64-bit). */
 static inline time_t

diff --git a/src/kadmin/server/ipropd_svc.c b/src/kadmin/server/ipropd_svc.c
index e5dd233e813ec98e246a5c482b87f58c410b8b7f..108c29e30d2038137e9d1589655abe5fe314d51c 100644 (file)
--- a/src/kadmin/server/ipropd_svc.c
+++ b/src/kadmin/server/ipropd_svc.c
@@ -122,7 +122,8 @@ buf_to_string(gss_buffer_desc *b)
     char *s = malloc(b->length+1);
 
     if (s) {
-       memcpy(s, b->value, b->length);
+       if (b->length > 0)
+           memcpy(s, b->value, b->length);
        s[b->length] = 0;
     }
     (void) gss_release_buffer(&min_stat, b);

diff --git a/src/kdc/fast_util.c b/src/kdc/fast_util.c
index 7a6579f1d2d43faa172e23f69e0607db54e0bdb0..848f80816b4f11a1b9b5807bf475db736e2c6354 100644 (file)
--- a/src/kdc/fast_util.c
+++ b/src/kdc/fast_util.c
@@ -453,7 +453,7 @@ make_padata(krb5_preauthtype pa_type, const void *contents, size_t len,
 {
     if (k5_alloc_pa_data(pa_type, len, out) != 0)
         return ENOMEM;
-    memcpy((*out)->contents, contents, len);
+    k5memcpy((*out)->contents, contents, len);
     return 0;
 }
 

diff --git a/src/kdc/rtest.c b/src/kdc/rtest.c
index 685228411dd8c41727a3106913a6121c115cbee3..06e442da97d5c56272f9c6eb1357cce08e8ae2eb 100644 (file)
--- a/src/kdc/rtest.c
+++ b/src/kdc/rtest.c
@@ -85,7 +85,7 @@ main(int argc, char **argv)
         otrans.data = (char *) malloc(otrans.length);
     else
         otrans.data = 0;
-    memcpy(otrans.data,argv[1], otrans.length);
+    k5memcpy(otrans.data,argv[1], otrans.length);
 
     tgs = make_princ(ctx, argv[2], argv[0]);
     cl  = make_princ(ctx, argv[3], argv[0]);

diff --git a/src/lib/crypto/builtin/kdf.c b/src/lib/crypto/builtin/kdf.c
index 8a6658bfde82d22835fdf37f4460d98e4b89ac74..8109d7c10b52cfc6396b06db45da5ae40bca436a 100644 (file)
--- a/src/lib/crypto/builtin/kdf.c
+++ b/src/lib/crypto/builtin/kdf.c
@@ -67,7 +67,7 @@ k5_sp800_108_counter_hmac(const struct krb5_hash_provider *hash,
 
     ret = krb5int_hmac(hash, key, iov, 5, &prf);
     if (!ret)
-        memcpy(rnd_out->data, prf.data, rnd_out->length);
+        k5memcpy(rnd_out->data, prf.data, rnd_out->length);
     zapfree(prf.data, prf.length);
     return ret;
 }

diff --git a/src/lib/crypto/krb/aead.c b/src/lib/crypto/krb/aead.c
index 9d4e206ab77cb7d570331fb78dd61769e02ee248..8a32204416477740124a1a167ebdcca4e5ce8249 100644 (file)
--- a/src/lib/crypto/krb/aead.c
+++ b/src/lib/crypto/krb/aead.c
@@ -179,7 +179,8 @@ k5_iov_cursor_get(struct iov_cursor *cursor, unsigned char *block)
         if (nbytes > remain)
             nbytes = remain;
 
-        memcpy(block + bsz - remain, iov->data.data + cursor->in_pos, nbytes);
+        k5memcpy(block + bsz - remain, iov->data.data + cursor->in_pos,
+                 nbytes);
         cursor->in_pos += nbytes;
         remain -= nbytes;
 
@@ -211,7 +212,8 @@ k5_iov_cursor_put(struct iov_cursor *cursor, unsigned char *block)
         if (nbytes > remain)
             nbytes = remain;
 
-        memcpy(iov->data.data + cursor->out_pos, block + bsz - remain, nbytes);
+        k5memcpy(iov->data.data + cursor->out_pos, block + bsz - remain,
+                 nbytes);
         cursor->out_pos += nbytes;
         remain -= nbytes;
 

diff --git a/src/lib/crypto/krb/cf2.c b/src/lib/crypto/krb/cf2.c
index 2ee5aebda3f52d77fa9cfc6fb06e2fb0c0458c92..5f86383f1834635178a32983d1c5e4f509f669a8 100644 (file)
--- a/src/lib/crypto/krb/cf2.c
+++ b/src/lib/crypto/krb/cf2.c
@@ -61,7 +61,7 @@ krb5_c_prfplus(krb5_context context, const krb5_keyblock *k,
 
     /* Concatenate PRF(k, 1||input) || PRF(k, 2||input) || ... to produce the
      * desired number of bytes. */
-    memcpy(&prf_in.data[1], input->data, input->length);
+    k5memcpy(&prf_in.data[1], input->data, input->length);
     for (i = 0; i < nblocks; i++) {
         prf_in.data[0] = i + 1;
         ret = krb5_c_prf(context, k, &prf_in, &prf_out);

diff --git a/src/lib/gssapi/krb5/prf.c b/src/lib/gssapi/krb5/prf.c
index f87957bdfb74a3730d5f072ab782961da750cab7..0f1b5baab0b7948f084b90a0a9c95ac84c7460ab 100644 (file)
--- a/src/lib/gssapi/krb5/prf.c
+++ b/src/lib/gssapi/krb5/prf.c
@@ -111,7 +111,7 @@ krb5_gss_pseudo_random(OM_uint32 *minor_status,
     if (t.data == NULL)
         goto cleanup;
 
-    memcpy(ns.data + 4, prf_in->value, prf_in->length);
+    k5memcpy(ns.data + 4, prf_in->value, prf_in->length);
     i = 0;
     p = (unsigned char *)prf_out->value;
     while (desired_output_len > 0) {

diff --git a/src/lib/krb5/asn.1/asn1_encode.c b/src/lib/krb5/asn.1/asn1_encode.c
index 7aebe60db222c1d0619d5d18f04beb307661ff96..6fdc3d4f2b84871cc0153ec557c062a459f64cf7 100644 (file)
--- a/src/lib/krb5/asn.1/asn1_encode.c
+++ b/src/lib/krb5/asn.1/asn1_encode.c
@@ -49,8 +49,8 @@ insert_byte(asn1buf *buf, uint8_t o)
 static inline void
 insert_bytes(asn1buf *buf, const void *bytes, size_t len)
 {
-    if (buf->ptr != NULL && len > 0) {
-        memcpy(buf->ptr - len, bytes, len);
+    if (buf->ptr != NULL) {
+        k5memcpy(buf->ptr - len, bytes, len);
         buf->ptr -= len;
     }
     buf->count += len;

diff --git a/src/lib/krb5/krb/serialize.c b/src/lib/krb5/krb/serialize.c
index 8934cf1548a75e03578c5ce71a6af885eea07ed9..e5aee839ba6b55bc3a69acdb54a2f62789526def 100644 (file)
--- a/src/lib/krb5/krb/serialize.c
+++ b/src/lib/krb5/krb/serialize.c
@@ -67,7 +67,7 @@ krb5_error_code KRB5_CALLCONV
 krb5_ser_pack_bytes(krb5_octet *ostring, size_t osize, krb5_octet **bufp, size_t *remainp)
 {
     if (*remainp >= osize) {
-        memcpy(*bufp, ostring, osize);
+        k5memcpy(*bufp, ostring, osize);
         *bufp += osize;
         *remainp -= osize;
         return(0);

diff --git a/src/lib/rpc/xdr_mem.c b/src/lib/rpc/xdr_mem.c
index 8d3f120171a0114e5606f1b438fc488731f2243e..ea8b76168255279ea96cae1da84cad7234f8fbef 100644 (file)
--- a/src/lib/rpc/xdr_mem.c
+++ b/src/lib/rpc/xdr_mem.c
@@ -129,7 +129,8 @@ xdrmem_getbytes(XDR *xdrs, caddr_t addr, u_int len)
                return (FALSE);
        else
                xdrs->x_handy -= len;
-       memmove(addr, xdrs->x_private, len);
+       if (len > 0)
+               memmove(addr, xdrs->x_private, len);
        xdrs->x_private = (char *)xdrs->x_private + len;
        return (TRUE);
 }
@@ -142,7 +143,8 @@ xdrmem_putbytes(XDR *xdrs, caddr_t addr, u_int len)
                return (FALSE);
        else
                xdrs->x_handy -= len;
-       memmove(xdrs->x_private, addr, len);
+       if (len > 0)
+               memmove(xdrs->x_private, addr, len);
        xdrs->x_private = (char *)xdrs->x_private + len;
        return (TRUE);
 }

diff --git a/src/plugins/kdb/db2/libdb2/btree/btree.h b/src/plugins/kdb/db2/libdb2/btree/btree.h
index 91fd271bf634f9762b9f3316318eccdfc3102771..37dc4fda1557c1ce00e89da756d1b775dc608ec7 100644 (file)
--- a/src/plugins/kdb/db2/libdb2/btree/btree.h
+++ b/src/plugins/kdb/db2/libdb2/btree/btree.h
@@ -196,9 +196,11 @@ typedef struct _bleaf {
        p += sizeof(u_int32_t);                                         \
        *(u_char *)p = flags;                                           \
        p += sizeof(u_char);                                            \
-       memmove(p, key->data, key->size);                               \
+       if (key->size > 0)                                              \
+           memmove(p, key->data, key->size);                           \
        p += key->size;                                                 \
-       memmove(p, data->data, data->size);                             \
+       if (data->size > 0)                                             \
+           memmove(p, data->data, data->size);                         \
 }
 
 /* For the recno leaf pages, the item is a data entry. */

diff --git a/src/plugins/kdb/ldap/libkdb_ldap/kdb_xdr.c b/src/plugins/kdb/ldap/libkdb_ldap/kdb_xdr.c
index ae51905ae86fae986e4fbdffa29a55dfbe8386b5..bd59aec6d459af54565c01585e12837d72de922f 100644 (file)
--- a/src/plugins/kdb/ldap/libkdb_ldap/kdb_xdr.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/kdb_xdr.c
@@ -73,7 +73,7 @@ krb5_dbe_update_tl_data(krb5_context context, krb5_db_entry *entry,
     tl_data->tl_data_type = new_tl_data->tl_data_type;
     tl_data->tl_data_length = new_tl_data->tl_data_length;
     tl_data->tl_data_contents = tmp;
-    memcpy(tmp, new_tl_data->tl_data_contents, tl_data->tl_data_length);
+    k5memcpy(tmp, new_tl_data->tl_data_contents, tl_data->tl_data_length);
 
     return(0);
 }

diff --git a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
index ae4e03f8cfb827e73f70c061cb011248617c236f..418d253d17cc5100e7f324e2de1652951500aa1a 100644 (file)
--- a/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
+++ b/src/plugins/kdb/ldap/libkdb_ldap/ldap_principal2.c
@@ -637,7 +637,7 @@ tl_data2berval (krb5_tl_data *in, struct berval **out)
     }
 
     STORE16_INT((*out)->bv_val, in->tl_data_type);
-    memcpy ((*out)->bv_val + 2, in->tl_data_contents, in->tl_data_length);
+    k5memcpy((*out)->bv_val + 2, in->tl_data_contents, in->tl_data_length);
 
     return 0;
 }

diff --git a/src/util/support/hashtab.c b/src/util/support/hashtab.c
index a66b8cb4383e29838d6fcdffb08c59940b99ff70..1a92ec0d281bfce0c0806fbca5b44e74aff9e4c9 100644 (file)
--- a/src/util/support/hashtab.c
+++ b/src/util/support/hashtab.c
@@ -94,7 +94,8 @@ siphash24(const uint8_t *data, size_t len, uint64_t k0, uint64_t k1)
     }
 
     /* Process the last 0-7 bytes followed by the length mod 256. */
-    memcpy(last, end, len % 8);
+    if (len % 8 > 0)
+        memcpy(last, end, len % 8);
     last[7] = len & 0xFF;
     mi = load_64_le(last);
     v3 ^= mi;