EdDSA verifier accepts non-canonical S plus the group order
Closed, ResolvedPublic

Assigned To
Authored By
• werner
Tue, Aug 4, 5:00 PM

Description

EdDSA verifier accepts non-canonical S plus the group order

The EdDSA verifier checks that encoded S has the expected byte length, converts it to an MPI, and immediately multiplies the base point. It never enforces the required scalar range S < L. Because adding the group order does not change S times the base point, a signature can have S replaced by S + L and still verify when the sum fits the encoding.

Vulnerable code

In cipher/ecc-eddsa.c, function _gcry_ecc_eddsa_verify:

_gcry_mpi_set_buffer (s, sbuf, slen, 0);
xfree (sbuf);
if (slen != b)
  {
    rc = GPG_ERR_INV_LENGTH;
    goto leave;
  }
}

_gcry_mpi_ec_mul_point (&Ia, s, ec->G, ec);

Why it matters

The supplied verification harness transformed a valid signature by adding L to S and observed success. This is signature malleability: it breaks systems that treat signature bytes as a unique identifier, use them for deduplication, or key policy decisions on exact encodings. It does not let an attacker sign a different message or forge without an existing valid signature.

Proposed fix

After decoding S and before scalar multiplication, reject S greater than or equal to the curve subgroup order ec->n with a bad-signature error. Add regression vectors for S equal to L, S + L, the largest canonical S, and valid Ed25519 and Ed448 signatures, ensuring noncanonical encodings are rejected consistently.

Related Objects

Event Timeline

• werner triaged this task as Normal priority.Tue, Aug 4, 5:00 PM
• werner created this task.
• werner created this object with visibility "Public (No Login Required)".
• werner created this object with edit policy "Contributor (Project)".
• gniibe raised the priority of this task from Normal to High.
• gniibe added a subscriber: • gniibe.

Here is the fix:

diff --git a/cipher/ecc-eddsa.c b/cipher/ecc-eddsa.c
index d3c6b6f2..8bfbde7b 100644
--- a/cipher/ecc-eddsa.c
+++ b/cipher/ecc-eddsa.c
@@ -1048,6 +1048,11 @@ _gcry_ecc_eddsa_verify (gcry_mpi_t input, mpi_ec_t ec,
         rc = GPG_ERR_INV_LENGTH;
         goto leave;
       }
+    if (mpi_cmp (s, ec->n) >= 0)
+      {
+        rc = GPG_ERR_BAD_SIGNATURE;
+        goto leave;
+      }
   }
 
   _gcry_mpi_ec_mul_point (&Ia, s, ec->G, ec);
• gniibe mentioned this in Unknown Object (Maniphest Task).Mon, Aug 10, 3:55 AM
• gniibe lowered the priority of this task from High to Normal.Tue, Aug 11, 8:43 AM
• gniibe changed the task status from Open to Testing.Wed, Aug 12, 4:53 AM
• werner shifted this object from the Restricted Space space to the S1 Public space.Tue, Aug 18, 2:47 PM
• gniibe mentioned this in Unknown Object (Maniphest Task).Mon, Aug 24, 8:26 AM