From 5909e20899334816f36cac0e47105e56df52ad3c Mon Sep 17 00:00:00 2001
From: William Brown <firstyear@redhat.com>
Date: Mon, 30 Oct 2017 12:01:34 +1000
Subject: [PATCH] Ticket 49424 - Resolve csiphash alignment issues

Bug Description:  On some platforms, uint64_t is not the same size
as a void * - as well, if the input is not aligned correctly, then
a number of nasty crashes can result

Fix Description:  Instead of relying on alignment to be correct,
we should memcpy the data to inputs instead.

https://pagure.io/389-ds-base/issue/49424

Author: wibrown

Review by: lslebodn, cgrzemba, vashirov, mreynolds (Thanks!)

(cherry picked from commit 751446440f5269a246e6e652a64e63aa5933734a)
---
 src/libsds/external/csiphash/csiphash.c | 52 +++++++++++++++++++--------------
 src/libsds/test/test_sds_csiphash.c     | 43 +++++++++++++++++++++------
 2 files changed, 64 insertions(+), 31 deletions(-)

diff --git a/src/libsds/external/csiphash/csiphash.c b/src/libsds/external/csiphash/csiphash.c
index 0089c82f7..2351db6cf 100644
--- a/src/libsds/external/csiphash/csiphash.c
+++ b/src/libsds/external/csiphash/csiphash.c
@@ -32,6 +32,9 @@
 #include <inttypes.h>
 #include <stddef.h> /* for size_t */
 
+#include <stdlib.h> /* calloc,free */
+#include <string.h> /* memcpy */
+
 #include <config.h>
 
 #if defined(HAVE_SYS_ENDIAN_H)
@@ -75,11 +78,24 @@
 uint64_t
 sds_siphash13(const void *src, size_t src_sz, const char key[16])
 {
-    const uint64_t *_key = (uint64_t *)key;
+    uint64_t _key[2] = {0};
+    memcpy(_key, key, 16);
     uint64_t k0 = _le64toh(_key[0]);
     uint64_t k1 = _le64toh(_key[1]);
     uint64_t b = (uint64_t)src_sz << 56;
-    const uint64_t *in = (uint64_t *)src;
+
+    size_t input_sz = (src_sz / sizeof(uint64_t)) + 1;
+
+    /* Account for non-uint64_t alligned input */
+    /* Could make this stack allocation */
+    uint64_t *in = calloc(1, input_sz * sizeof(uint64_t));
+    /*
+     * Because all crypto code sucks, they modify *in
+     * during operation, so we stash a copy of the ptr here.
+     * alternately, we could use stack allocated array, but gcc
+     * will complain about the vla being unbounded.
+     */
+    uint64_t *in_ptr = memcpy(in, src, src_sz);
 
     uint64_t v0 = k0 ^ 0x736f6d6570736575ULL;
     uint64_t v1 = k1 ^ 0x646f72616e646f6dULL;
@@ -96,27 +112,15 @@ sds_siphash13(const void *src, size_t src_sz, const char key[16])
         v0 ^= mi;
     }
 
+    /*
+     * Because we allocate in as size + 1, we can over-read 0
+     * for this buffer to be padded correctly. in here is a pointer to the
+     * excess data because the while loop above increments the in pointer
+     * to point to the excess once src_sz drops < 8.
+     */
     uint64_t t = 0;
-    uint8_t *pt = (uint8_t *)&t;
-    uint8_t *m = (uint8_t *)in;
-
-    switch (src_sz) {
-    case 7:
-        pt[6] = m[6]; /* FALLTHRU */
-    case 6:
-        pt[5] = m[5]; /* FALLTHRU */
-    case 5:
-        pt[4] = m[4]; /* FALLTHRU */
-    case 4:
-        *((uint32_t *)&pt[0]) = *((uint32_t *)&m[0]);
-        break;
-    case 3:
-        pt[2] = m[2]; /* FALLTHRU */
-    case 2:
-        pt[1] = m[1]; /* FALLTHRU */
-    case 1:
-        pt[0] = m[0]; /* FALLTHRU */
-    }
+    memcpy(&t, in, sizeof(uint64_t));
+
     b |= _le64toh(t);
 
     v3 ^= b;
@@ -126,5 +130,9 @@ sds_siphash13(const void *src, size_t src_sz, const char key[16])
     v2 ^= 0xff;
     // dround
     dROUND(v0, v1, v2, v3);
+
+    free(in_ptr);
+
     return (v0 ^ v1) ^ (v2 ^ v3);
 }
+
diff --git a/src/libsds/test/test_sds_csiphash.c b/src/libsds/test/test_sds_csiphash.c
index cdb6b7f46..cc9a6b2b5 100644
--- a/src/libsds/test/test_sds_csiphash.c
+++ b/src/libsds/test/test_sds_csiphash.c
@@ -25,23 +25,48 @@
 static void
 test_siphash(void **state __attribute__((unused)))
 {
-
-    //
     uint64_t value = 0;
     uint64_t hashout = 0;
     char key[16] = {0};
 
-    uint64_t test_a = 15794382300316794652U;
-    uint64_t test_b = 13042610424265326907U;
+    uint64_t test_simple = 15794382300316794652U;
 
-    // Initial simple test
+    /* Initial simple test */
     value = htole64(5);
     hashout = sds_siphash13(&value, sizeof(uint64_t), key);
-    assert_true(hashout == test_a);
+    assert_int_equal(hashout, test_simple);
+
+    /* Test a range of input sizes to check endianness behaviour */
+
+    hashout = sds_siphash13("a", 1, key);
+    assert_int_equal(hashout, 0x407448d2b89b1813U);
+
+    hashout = sds_siphash13("aa", 2, key);
+    assert_int_equal(hashout, 0x7910e0436ed8d1deU);
+
+    hashout = sds_siphash13("aaa", 3, key);
+    assert_int_equal(hashout, 0xf752893a6c769652U);
+
+    hashout = sds_siphash13("aaaa", 4, key);
+    assert_int_equal(hashout, 0x8b02350718d87164U);
+
+    hashout = sds_siphash13("aaaaa", 5, key);
+    assert_int_equal(hashout, 0x92a991474c7eef2U);
+
+    hashout = sds_siphash13("aaaaaa", 6, key);
+    assert_int_equal(hashout, 0xf0ab815a640277ccU);
+
+    hashout = sds_siphash13("aaaaaaa", 7, key);
+    assert_int_equal(hashout, 0x33f3c6d7dbc82c0dU);
+
+    hashout = sds_siphash13("aaaaaaaa", 8, key);
+    assert_int_equal(hashout, 0xc501b12e18428c92U);
+
+    hashout = sds_siphash13("aaaaaaaabbbb", 12, key);
+    assert_int_equal(hashout, 0xcddca673069ade64U);
 
-    char *test = "abc";
-    hashout = sds_siphash13(test, 4, key);
-    assert_true(hashout == test_b);
+    hashout = sds_siphash13("aaaaaaaabbbbbbbb", 16, key);
+    assert_int_equal(hashout, 0xdc54f0bfc0e1deb0U);
 }
 
 int
-- 
2.13.6