ecc: No check for broken public key when verify signature (ECDSA, ECDSA for SM and GOST)
Closed, ResolvedPublic
Actions

Assigned To

Authored By

	• gniibe
	Feb 1 2021, 8:13 AM

Description

My fuzzer found this:

ecc curve: secp256r1
public key X:
4534198767316794591643245143622298809742628679895448054572722918996032022405
public key Y:
107839128084157537346759045080774377135290251058561962283882310383644151460337
cleartext: {0xff, 0xff, 0xff, 0xff, 0x00, 0x00, 0x00, 0x00, 0xff, 0xff,
0xff, 0xff, 0xff, 0xff, 0xff, 0xff,
0xbc, 0xe6, 0xfa, 0xad, 0xa7, 0x17, 0x9e, 0x84, 0xf3, 0xb9, 0xca, 0xc2,
0xfc, 0x63, 0x25, 0x51} (32 bytes)
signature R:
4534198767316794591643245143622298809742628679895448054572722918996032022405
signature S:
4534198767316794591643245143622298809742628679895448054572722918996032022405

where 'cleartext' is the data passed as-is (unhashed) to the verification
function.

gcry_pk_verify() returns GPG_ERR_NO_ERROR for these parameters but other
libraries return failure.

Details

External Link: https://lists.gnupg.org/pipermail/gcrypt-devel/2021-January/005091.html

Revisions and Commits

rC libgcrypt
	rC1d312bc65846 ecc: Add checking key for ECDSA.
	rC598d0f3e0294 ecc: Add checking key for ECDSA.

Related Objects

Mentioned In: T5466: Release Libgcrypt 1.8.8
T5276: Release Libgcrypt 1.9.2
Mentioned Here: rC1d312bc65846: ecc: Add checking key for ECDSA.

Event Timeline

Here is a patch adding those checks:

diff --git a/cipher/ecc-ecdsa.c b/cipher/ecc-ecdsa.c
index d540578e..30103f14 100644
--- a/cipher/ecc-ecdsa.c
+++ b/cipher/ecc-ecdsa.c
@@ -172,6 +172,9 @@ _gcry_ecc_ecdsa_verify (gcry_mpi_t input, mpi_ec_t ec,
   mpi_point_struct Q, Q1, Q2;
   unsigned int nbits;
 
+  if (!_gcry_mpi_ec_curve_point (ec->Q, ec))
+    return GPG_ERR_BROKEN_PUBKEY;
+
   if( !(mpi_cmp_ui (r, 0) > 0 && mpi_cmp (r, ec->n) < 0) )
     return GPG_ERR_BAD_SIGNATURE; /* Assertion	0 < r < n  failed.  */
   if( !(mpi_cmp_ui (s, 0) > 0 && mpi_cmp (s, ec->n) < 0) )
diff --git a/cipher/ecc-gost.c b/cipher/ecc-gost.c
index c5ab774d..36230f8a 100644
--- a/cipher/ecc-gost.c
+++ b/cipher/ecc-gost.c
@@ -132,6 +132,9 @@ _gcry_ecc_gost_verify (gcry_mpi_t input, mpi_ec_t ec,
   gcry_mpi_t e, x, z1, z2, v, rv, zero;
   mpi_point_struct Q, Q1, Q2;
 
+  if (!_gcry_mpi_ec_curve_point (ec->Q, ec))
+    return GPG_ERR_BROKEN_PUBKEY;
+
   if( !(mpi_cmp_ui (r, 0) > 0 && mpi_cmp (r, ec->n) < 0) )
     return GPG_ERR_BAD_SIGNATURE; /* Assertion	0 < r < n  failed.  */
   if( !(mpi_cmp_ui (s, 0) > 0 && mpi_cmp (s, ec->n) < 0) )
diff --git a/cipher/ecc-sm2.c b/cipher/ecc-sm2.c
index 135c7697..c52629fd 100644
--- a/cipher/ecc-sm2.c
+++ b/cipher/ecc-sm2.c
@@ -500,6 +500,9 @@ _gcry_ecc_sm2_verify (gcry_mpi_t input, mpi_ec_t ec,
   gcry_mpi_t x1, y1;
   unsigned int nbits;
 
+  if (!_gcry_mpi_ec_curve_point (ec->Q, ec))
+    return GPG_ERR_BROKEN_PUBKEY;
+
   /* r, s within [1, n-1] */
   if (mpi_cmp_ui (r, 1) < 0 || mpi_cmp (r, ec->n) > 0)
     return GPG_ERR_BAD_SIGNATURE;

I think that a backport to 1.8. also makes sense

• gniibe added a commit: rC598d0f3e0294: ecc: Add checking key for ECDSA..Feb 2 2021, 2:26 AM

Backport was done with commit rC1d312bc65846 (for unknown reasons it did not show up in the list of bugs related to this bug; I added it by hand). Fix will go into 1.8.8.

• werner removed a project: backport.Feb 17 2021, 8:53 AM

• werner mentioned this in T5276: Release Libgcrypt 1.9.2.Feb 17 2021, 9:44 AM

• werner mentioned this in T5466: Release Libgcrypt 1.8.8.Jun 2 2021, 2:40 PM