[glib: 1/3] ghmac: Fix some signed/unsigned issues with g_checksum_update()




commit a3911ef1590c6dceea668d366519b9a7798ff5a1
Author: Philip Withnall <pwithnall endlessos org>
Date:   Thu May 5 13:18:40 2022 +0100

    ghmac: Fix some signed/unsigned issues with g_checksum_update()
    
    The length argument to `g_checksum_update()` is signed, allowing
    `length < 0` to indicate a nul-terminated input string. However, most of
    the `GHmac` machinery which calls `g_checksum_update()` uses unsigned
    `gsize`s.
    
    If any of those sizes exceed `G_MAXSSIZE` (which is very unlikely and
    could only happen with a buggy caller), the unsigned-to-signed
    conversion would wrap and cause `g_checksum_update()` to inappropriately
    interpret the input as nul-terminated.
    
    Fix that by adding a load of assertions and making the
    unsigned-to-signed comparisons explicit.
    
    Signed-off-by: Philip Withnall <pwithnall endlessos org>
    
    Coverity CID: #1486807

 glib/ghmac.c | 20 ++++++++++++++++----
 1 file changed, 16 insertions(+), 4 deletions(-)
---
diff --git a/glib/ghmac.c b/glib/ghmac.c
index 54da9f9361..96c7dedb11 100644
--- a/glib/ghmac.c
+++ b/glib/ghmac.c
@@ -100,6 +100,9 @@ g_hmac_new (GChecksumType  digest_type,
   guchar *pad;
   gsize i, len;
   gsize block_size;
+  gssize block_size_signed, key_len_signed;
+
+  g_return_val_if_fail (key_len <= G_MAXSSIZE, NULL);
 
   checksum = g_checksum_new (digest_type);
   g_return_val_if_fail (checksum != NULL, NULL);
@@ -134,7 +137,9 @@ g_hmac_new (GChecksumType  digest_type,
   if (key_len > block_size)
     {
       len = block_size;
-      g_checksum_update (hmac->digesti, key, key_len);
+      g_assert (key_len <= G_MAXSSIZE);
+      key_len_signed = key_len;
+      g_checksum_update (hmac->digesti, key, key_len_signed);
       g_checksum_get_digest (hmac->digesti, buffer, &len);
       g_checksum_reset (hmac->digesti);
     }
@@ -145,15 +150,19 @@ g_hmac_new (GChecksumType  digest_type,
       memcpy (buffer, key, key_len);
     }
 
+  /* g_checksum_update() accepts a signed length, so build and check that. */
+  g_assert (block_size <= G_MAXSSIZE);
+  block_size_signed = block_size;
+
   /* First pad */
   for (i = 0; i < block_size; i++)
     pad[i] = 0x36 ^ buffer[i]; /* ipad value */
-  g_checksum_update (hmac->digesti, pad, block_size);
+  g_checksum_update (hmac->digesti, pad, block_size_signed);
 
   /* Second pad */
   for (i = 0; i < block_size; i++)
     pad[i] = 0x5c ^ buffer[i]; /* opad value */
-  g_checksum_update (hmac->digesto, pad, block_size);
+  g_checksum_update (hmac->digesto, pad, block_size_signed);
 
   return hmac;
 }
@@ -316,6 +325,7 @@ g_hmac_get_digest (GHmac  *hmac,
                    gsize  *digest_len)
 {
   gsize len;
+  gssize len_signed;
 
   g_return_if_fail (hmac != NULL);
 
@@ -324,7 +334,9 @@ g_hmac_get_digest (GHmac  *hmac,
 
   /* Use the same buffer, because we can :) */
   g_checksum_get_digest (hmac->digesti, buffer, &len);
-  g_checksum_update (hmac->digesto, buffer, len);
+  g_assert (len <= G_MAXSSIZE);
+  len_signed = len;
+  g_checksum_update (hmac->digesto, buffer, len_signed);
   g_checksum_get_digest (hmac->digesto, buffer, digest_len);
 }
 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]