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