[PATCH 0/2] Fixes for gcry_cipher_gettag crashes

[Werner Koch]

On Wed, 23 Mar 2016 03:45, peter at lekensteyn.nl said:

> Poly1305 was introduced this cycle (1.7.0), but GCM was introduced in 1.6.0.
> Maybe a backport is appropriate?

Definitely.  I'll do that soon.

> Peter Wu (2):
>   Fix buffer overrun in gettag for GCM
>   Fix buffer overrun in gettag for Poly1305

I applied them but also added this additional fix:

--8<---------------cut here---------------start------------->8---
    cipher: Check length of supplied tag in _gcry_cipher_gcm_check_tag.
    
    * cipher/cipher-gcm.c (_gcry_cipher_gcm_tag): Check that the provided
    tag length matches the actual tag length.  Avoid gratuitous return
    statements.
    --
    
    Signed-off-by: Werner Koch <wk at gnupg.org>

	Modified   cipher/cipher-gcm.c
diff --git a/cipher/cipher-gcm.c b/cipher/cipher-gcm.c
index cb81ea9..5e9dec4 100644
--- a/cipher/cipher-gcm.c
+++ b/cipher/cipher-gcm.c
@@ -803,13 +803,18 @@ _gcry_cipher_gcm_tag (gcry_cipher_hd_t c,
 
   if (!check)
     {
+      /* NB: We already checked that OUTBUF is large enough to hold
+         the result.  */
       memcpy (outbuf, c->u_mode.gcm.u_tag.tag, GCRY_GCM_BLOCK_LEN);
-      return GPG_ERR_NO_ERROR;
     }
   else
     {
-      return buf_eq_const(outbuf, c->u_mode.gcm.u_tag.tag, GCRY_GCM_BLOCK_LEN) ?
-               GPG_ERR_NO_ERROR : GPG_ERR_CHECKSUM;
+      /* OUTBUFLEN gives the length of the user supplied tag in OUTBUF
+       * and thus we need to compare its length first.  */
+      if (outbuflen != GCRY_GCM_BLOCK_LEN
+          || !buf_eq_const (outbuf, c->u_mode.gcm.u_tag.tag,
+                            GCRY_GCM_BLOCK_LEN))
+        return GPG_ERR_CHECKSUM;
     }
--8<---------------cut here---------------end--------------->8---

The same goes for poly1305.


Salam-Shalom,

   Werner

-- 
Die Gedanken sind frei.  Ausnahmen regelt ein Bundesgesetz.