OAEP size-check underflow permits heap writes past tiny RSA frames
Closed, ResolvedPublic

Assigned To
Authored By
werner
Tue, Aug 4, 4:45 PM

Description

OAEP size-check underflow permits heap writes past tiny RSA frames

The OAEP encoder subtracts its fixed overhead from an unsigned frame length before proving that the overhead fits. A public gcry_pk_encrypt call using a supported 1024-bit RSA key and SHA-512 therefore accepts a one-byte message, allocates 128 bytes, and writes a 64-byte digest starting at offset 65 -- one byte beyond the secure allocation.

Vulnerable code

cipher/rsa-common.c, _gcry_rsa_oaep_encode:

c
  hlen = _gcry_md_get_algo_dlen (algo);

  /* We skip step 1a which would be to check that LABELLEN is not
     greater than 2^61-1.  See rfc-3447 7.1.1. */

  /* Step 1b.  Note that the obsolete rfc-2437 uses the check:
     valuelen > nframe - 2 * hlen - 1 .  */
  if (valuelen > nframe - 2 * hlen - 2 || !nframe)
    {
      /* Can't encode a VALUELEN value in a NFRAME bytes frame. */
      return GPG_ERR_TOO_SHORT; /* The key is too short.  */
    }

  /* Allocate the frame.  */
  frame = xtrycalloc_secure (1, nframe);
  if (!frame)
    return gpg_err_code_from_syserror ();

  /* Step 2a: Compute the hash of the label.  We store it in the frame
     where later the maskedDB will commence.  */
  _gcry_md_hash_buffer (algo, frame + 1 + hlen, label, labellen);

Why it matters

Applications commonly encrypt to externally supplied public keys. Outside FIPS mode, 1024-bit RSA remains accepted, so no malformed tiny modulus is required: OAEP/SHA-512 needs at least 130 frame bytes, but a 1024-bit modulus supplies 128. The audit's public-API ASan reproducer reported a 64-byte heap-buffer-overflow in _gcry_md_hash_buffer; the same test safely returned GPG_ERR_TOO_SHORT after reordering the check.

Proposed fix

Compute overhead = 2 * hlen + 2 with checked size_t arithmetic, reject nframe < overhead, and only then evaluate valuelen > nframe - overhead. Add public-API regression cases for every supported OAEP digest at frame sizes immediately below, at, and above its minimum, including 1024-bit RSA with SHA-512.

Related Objects

Event Timeline

werner created this task.
werner created this object with visibility "Public (No Login Required)".
werner created this object with edit policy "Contributor (Project)".
werner renamed this task from oaep size underflow overflows rsa frame to OAEP size-check underflow permits heap writes past tiny RSA frames.Tue, Aug 4, 5:12 PM

Same issue is also reported by: X1AOxiang <xiao__xiang@163.com>

Here is a fix:

diff --git a/cipher/rsa-common.c b/cipher/rsa-common.c
index b6ae1d93..9b7442ee 100644
--- a/cipher/rsa-common.c
+++ b/cipher/rsa-common.c
@@ -529,6 +529,11 @@ _gcry_rsa_oaep_encode (gcry_mpi_t *r_result, unsigned int nbits, int algo,
 
   hlen = _gcry_md_get_algo_dlen (algo);
 
+  /* Public key check against hash algo */
+  /* "Step 0", so to say.  (It's implicit in rfc-3447/8017).  */
+  if (nframe < 2 * hlen + 2)
+    return GPG_ERR_DIGEST_ALGO;
+
   /* We skip step 1a which would be to check that LABELLEN is not
      greater than 2^61-1.  See rfc-3447 7.1.1. */

Suggested fix says about 2 * hlen and + 2 should be careful. I use assumptions where existing hash algorithms (or future ones) don't produce such a larger result, to omit runtime checks.

In general, I think that it's the application responsibility to validate key and algo before using pubkey API of libgcrypt.
It would be kind when erroneous use cases are rejected, though.

gniibe mentioned this in Unknown Object (Maniphest Task).Mon, Aug 10, 3:55 AM
gniibe changed the task status from Open to Testing.Tue, Aug 11, 4:28 AM
gniibe lowered the priority of this task from High to Normal.
gniibe mentioned this in Unknown Object (Maniphest Task).Mon, Aug 24, 8:26 AM
werner shifted this object from the Restricted Space space to the S1 Public space.