[PATCH 3/3] crypto: add preventative measures wrt uninitialized hash info

["Daniel P. Berrangé" <berrange@redhat.com>]

Several static arrays can create incorrect behaviour if a new hash
algorithm is added without them being explicitly initialized. Be
more robust against this problem by using a static assert for the
array size, instead of predeclaring the array size.

When querying the arrays also add further sanity checks for missing
initializers.

Fixes: https://gitlab.com/qemu-project/qemu/-/work_items/4786
Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 crypto/hash-glib.c | 18 ++++++++++++++++--
 crypto/hash.c      | 12 +++++++++++-
 crypto/hmac-glib.c | 18 ++++++++++++++++--
 3 files changed, 43 insertions(+), 5 deletions(-)

diff --git a/crypto/hash-glib.c b/crypto/hash-glib.c
index 39040f1856e..d83caec8cab 100644
--- a/crypto/hash-glib.c
+++ b/crypto/hash-glib.c
@@ -25,7 +25,7 @@
 #include "hashpriv.h"
 
 
-static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
+static int qcrypto_hash_alg_map[] = {
     [QCRYPTO_HASH_ALGO_MD5] = G_CHECKSUM_MD5,
     [QCRYPTO_HASH_ALGO_SHA1] = G_CHECKSUM_SHA1,
     [QCRYPTO_HASH_ALGO_SHA224] = -1,
@@ -35,11 +35,25 @@ static int qcrypto_hash_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
     [QCRYPTO_HASH_ALGO_RIPEMD160] = -1,
     [QCRYPTO_HASH_ALGO_SM3] = -1,
 };
+/*
+ * Prefer the assert over declaring the array with
+ * size QCRYPTO_HASH_ALGO__MAX so we get better
+ * detection of uninitialize elements.
+ */
+G_STATIC_ASSERT(G_N_ELEMENTS(qcrypto_hash_alg_map) ==
+                QCRYPTO_HASH_ALGO__MAX);
 
 gboolean qcrypto_hash_supports(QCryptoHashAlgo alg)
 {
+    /*
+     * The default static initializer value of 0
+     * matches G_CHECKSUM_MD5, so protect against
+     * accidentally uninitialized fields
+     */
     if (alg < G_N_ELEMENTS(qcrypto_hash_alg_map) &&
-        qcrypto_hash_alg_map[alg] != -1) {
+        qcrypto_hash_alg_map[alg] != -1 &&
+        (alg == QCRYPTO_HASH_ALGO_MD5 ||
+         qcrypto_hash_alg_map[alg] != G_CHECKSUM_MD5)) {
         return true;
     }
     return false;
diff --git a/crypto/hash.c b/crypto/hash.c
index 42a0a8ec56f..e966fc1a633 100644
--- a/crypto/hash.c
+++ b/crypto/hash.c
@@ -25,7 +25,7 @@
 #include "crypto/hash.h"
 #include "hashpriv.h"
 
-static size_t qcrypto_hash_alg_size[QCRYPTO_HASH_ALGO__MAX] = {
+static size_t qcrypto_hash_alg_size[] = {
     [QCRYPTO_HASH_ALGO_MD5]       = QCRYPTO_HASH_DIGEST_LEN_MD5,
     [QCRYPTO_HASH_ALGO_SHA1]      = QCRYPTO_HASH_DIGEST_LEN_SHA1,
     [QCRYPTO_HASH_ALGO_SHA224]    = QCRYPTO_HASH_DIGEST_LEN_SHA224,
@@ -35,10 +35,20 @@ static size_t qcrypto_hash_alg_size[QCRYPTO_HASH_ALGO__MAX] = {
     [QCRYPTO_HASH_ALGO_RIPEMD160] = QCRYPTO_HASH_DIGEST_LEN_RIPEMD160,
     [QCRYPTO_HASH_ALGO_SM3]       = QCRYPTO_HASH_DIGEST_LEN_SM3,
 };
+/*
+ * Prefer the assert over declaring the array with
+ * size QCRYPTO_HASH_ALGO__MAX so we get better
+ * detection of uninitialized elements. We don't
+ * want to ever return digest lengths of zero, as
+ * that risks a div-by-zero in callers.
+ */
+G_STATIC_ASSERT(G_N_ELEMENTS(qcrypto_hash_alg_size) ==
+                QCRYPTO_HASH_ALGO__MAX);
 
 size_t qcrypto_hash_digest_len(QCryptoHashAlgo alg)
 {
     assert(alg < G_N_ELEMENTS(qcrypto_hash_alg_size));
+    assert(qcrypto_hash_alg_size[alg] != 0);
     return qcrypto_hash_alg_size[alg];
 }
 
diff --git a/crypto/hmac-glib.c b/crypto/hmac-glib.c
index 8f652c5064e..34090444083 100644
--- a/crypto/hmac-glib.c
+++ b/crypto/hmac-glib.c
@@ -17,7 +17,7 @@
 #include "crypto/hmac.h"
 #include "hmacpriv.h"
 
-static int qcrypto_hmac_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
+static int qcrypto_hmac_alg_map[] = {
     [QCRYPTO_HASH_ALGO_MD5] = G_CHECKSUM_MD5,
     [QCRYPTO_HASH_ALGO_SHA1] = G_CHECKSUM_SHA1,
     [QCRYPTO_HASH_ALGO_SHA256] = G_CHECKSUM_SHA256,
@@ -27,6 +27,13 @@ static int qcrypto_hmac_alg_map[QCRYPTO_HASH_ALGO__MAX] = {
     [QCRYPTO_HASH_ALGO_RIPEMD160] = -1,
     [QCRYPTO_HASH_ALGO_SM3] = -1,
 };
+/*
+ * Prefer the assert over declaring the array with
+ * size QCRYPTO_HASH_ALGO__MAX so we get better
+ * detection of uninitialize elements.
+ */
+G_STATIC_ASSERT(G_N_ELEMENTS(qcrypto_hmac_alg_map) ==
+                QCRYPTO_HASH_ALGO__MAX);
 
 typedef struct QCryptoHmacGlib QCryptoHmacGlib;
 struct QCryptoHmacGlib {
@@ -35,8 +42,15 @@ struct QCryptoHmacGlib {
 
 bool qcrypto_hmac_supports(QCryptoHashAlgo alg)
 {
+    /*
+     * The default static initializer value of 0
+     * matches G_CHECKSUM_MD5, so protect against
+     * accidentally uninitialized fields
+     */
     if (alg < G_N_ELEMENTS(qcrypto_hmac_alg_map) &&
-        qcrypto_hmac_alg_map[alg] != -1) {
+        qcrypto_hmac_alg_map[alg] != -1 &&
+        (alg == QCRYPTO_HASH_ALGO_MD5 ||
+         qcrypto_hmac_alg_map[alg] != G_CHECKSUM_MD5)) {
         return true;
     }
 
-- 
2.56.0