Merge pull request #1703 from o1-labs/fix/check-ecdsa-foreignfield

Add multi-range check to `assertAlmostReduced`

CHANGELOG.md

@@ -29,6 +29,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Deprecated
 
 - `MerkleMap.computeRootAndKey()` deprecated in favor of `MerkleMap.computeRootAndKeyV2()` due to a potential issue of computing hash collisions in key indicies https://github.com/o1-labs/o1js/pull/1694
+- `createEcdsa`, `createForeignCurve`, `ForeignCurve` and `EcdsaSignature` deprecated in favor of `V2` versions due to a security vulnerability found in the current implementation https://github.com/o1-labs/o1js/pull/1703
 
 ## [1.3.1](https://github.com/o1-labs/o1js/compare/1ad7333e9e...40c597775) - 2024-06-11
 

src/examples/circuit/ecdsa.ts

@@ -3,16 +3,16 @@ import {
   circuitMain,
   public_,
   Crypto,
-  createEcdsa,
-  createForeignCurve,
+  createEcdsaV2,
+  createForeignCurveV2,
   Bytes,
   assert,
 } from 'o1js';
 
 export { Secp256k1, Ecdsa, Bytes32, Reserves };
 
-class Secp256k1 extends createForeignCurve(Crypto.CurveParams.Secp256k1) {}
-class Ecdsa extends createEcdsa(Secp256k1) {}
+class Secp256k1 extends createForeignCurveV2(Crypto.CurveParams.Secp256k1) {}
+class Ecdsa extends createEcdsaV2(Secp256k1) {}
 class Bytes32 extends Bytes(32) {}
 
 class Reserves extends Circuit {

src/examples/crypto/ecdsa/ecdsa.ts

@@ -2,14 +2,14 @@ import {
   ZkProgram,
   Crypto,
   createEcdsa,
-  createForeignCurve,
+  createForeignCurveV2,
   Bool,
   Bytes,
 } from 'o1js';
 
 export { keccakAndEcdsa, ecdsa, Secp256k1, Ecdsa, Bytes32 };
 
-class Secp256k1 extends createForeignCurve(Crypto.CurveParams.Secp256k1) {}
+class Secp256k1 extends createForeignCurveV2(Crypto.CurveParams.Secp256k1) {}
 class Scalar extends Secp256k1.Scalar {}
 class Ecdsa extends createEcdsa(Secp256k1) {}
 class Bytes32 extends Bytes(32) {}

src/index.ts

@@ -9,11 +9,15 @@ export {
 } from './lib/provable/foreign-field.js';
 export {
   createForeignCurve,
+  createForeignCurveV2,
   ForeignCurve,
+  ForeignCurveV2,
 } from './lib/provable/crypto/foreign-curve.js';
 export {
   createEcdsa,
+  createEcdsaV2,
   EcdsaSignature,
+  EcdsaSignatureV2,
 } from './lib/provable/crypto/foreign-ecdsa.js';
 export {
   Poseidon,

src/lib/provable/crypto/foreign-curve.ts

@@ -11,9 +11,15 @@ import { Field3 } from '../gadgets/foreign-field.js';
 import { assert } from '../gadgets/common.js';
 import { Provable } from '../provable.js';
 import { provableFromClass } from '../types/provable-derivers.js';
+import { multiRangeCheck } from '../gadgets/range-check.js';
 
 // external API
-export { createForeignCurve, ForeignCurve };
+export {
+  createForeignCurve,
+  createForeignCurveV2,
+  ForeignCurve,
+  ForeignCurveV2,
+};
 
 // internal API
 export { toPoint, FlexiblePoint };
@@ -26,6 +32,9 @@ function toPoint({ x, y }: ForeignCurve): Point {
   return { x: x.value, y: y.value };
 }
 
+/**
+ * @deprecated `ForeignCurve` is now deprecated and will be removed in a future release. Please use {@link ForeignCurveV2} instead.
+ */
 class ForeignCurve {
   x: AlmostForeignField;
   y: AlmostForeignField;
@@ -276,6 +285,48 @@ class ForeignCurve {
   }
 }
 
+class ForeignCurveV2 extends ForeignCurve {
+  constructor(g: {
+    x: AlmostForeignField | Field3 | bigint | number;
+    y: AlmostForeignField | Field3 | bigint | number;
+  }) {
+    super(g);
+  }
+
+  static check(g: ForeignCurveV2) {
+    multiRangeCheck(g.x.value);
+    multiRangeCheck(g.y.value);
+    // more efficient than the automatic check, which would do this for each field separately
+    this.Field.assertAlmostReduced(g.x, g.y);
+    this.assertOnCurve(g);
+    this.assertInSubgroup(g);
+  }
+}
+
+/**
+ * @deprecated `createForeignCurve` is now deprecated and will be removed in a future release. Please use {@link createForeignCurveV2} instead.
+ */
+function createForeignCurve(params: CurveParams): typeof ForeignCurve {
+  const FieldUnreduced = createForeignField(params.modulus);
+  const ScalarUnreduced = createForeignField(params.order);
+  class Field extends FieldUnreduced.AlmostReduced {}
+  class Scalar extends ScalarUnreduced.AlmostReduced {}
+
+  const BigintCurve = createCurveAffine(params);
+
+  class Curve extends ForeignCurve {
+    static _Bigint = BigintCurve;
+    static _Field = Field;
+    static _Scalar = Scalar;
+    static _provable = provableFromClass(Curve, {
+      x: Field.provable,
+      y: Field.provable,
+    });
+  }
+
+  return Curve;
+}
+
 /**
  * Create a class representing an elliptic curve group, which is different from the native {@link Group}.
  *
@@ -286,20 +337,20 @@ class ForeignCurve {
  * `createForeignCurve(params)` takes curve parameters {@link CurveParams} as input.
  * We support `modulus` and `order` to be prime numbers up to 259 bits.
  *
- * The returned {@link ForeignCurve} class represents a _non-zero curve point_ and supports standard
+ * The returned {@link ForeignCurveV2} class represents a _non-zero curve point_ and supports standard
  * elliptic curve operations like point addition and scalar multiplication.
  *
- * {@link ForeignCurve} also includes to associated foreign fields: `ForeignCurve.Field` and `ForeignCurve.Scalar`, see {@link createForeignField}.
+ * {@link ForeignCurveV2} also includes to associated foreign fields: `ForeignCurve.Field` and `ForeignCurve.Scalar`, see {@link createForeignFieldV2}.
  */
-function createForeignCurve(params: CurveParams): typeof ForeignCurve {
+function createForeignCurveV2(params: CurveParams): typeof ForeignCurveV2 {
   const FieldUnreduced = createForeignField(params.modulus);
   const ScalarUnreduced = createForeignField(params.order);
   class Field extends FieldUnreduced.AlmostReduced {}
   class Scalar extends ScalarUnreduced.AlmostReduced {}
 
   const BigintCurve = createCurveAffine(params);
 
-  class Curve extends ForeignCurve {
+  class Curve extends ForeignCurveV2 {
     static _Bigint = BigintCurve;
     static _Field = Field;
     static _Scalar = Scalar;

src/lib/provable/crypto/foreign-ecdsa.ts

@@ -4,20 +4,23 @@ import { ProvablePureExtended } from '../types/struct.js';
 import {
   FlexiblePoint,
   ForeignCurve,
+  ForeignCurveV2,
   createForeignCurve,
+  createForeignCurveV2,
   toPoint,
 } from './foreign-curve.js';
 import { AlmostForeignField } from '../foreign-field.js';
 import { assert } from '../gadgets/common.js';
 import { Field3 } from '../gadgets/foreign-field.js';
 import { Ecdsa } from '../gadgets/elliptic-curve.js';
-import { l } from '../gadgets/range-check.js';
+import { l, multiRangeCheck } from '../gadgets/range-check.js';
 import { Keccak } from './keccak.js';
 import { Bytes } from '../wrapped-classes.js';
 import { UInt8 } from '../int.js';
+import type { Bool } from '../bool.js';
 
 // external API
-export { createEcdsa, EcdsaSignature };
+export { createEcdsa, createEcdsaV2, EcdsaSignature, EcdsaSignatureV2 };
 
 type FlexibleSignature =
   | EcdsaSignature
@@ -26,6 +29,9 @@ type FlexibleSignature =
       s: AlmostForeignField | Field3 | bigint | number;
     };
 
+/**
+ * @deprecated `EcdsaSignature` is now deprecated and will be removed in a future release. Please use {@link EcdsaSignatureV2} instead.
+ */
 class EcdsaSignature {
   r: AlmostForeignField;
   s: AlmostForeignField;
@@ -69,7 +75,7 @@ class EcdsaSignature {
   /**
    * @deprecated There is a security vulnerability in this method. Use {@link verifyV2} instead.
    */
-  verify(message: Bytes, publicKey: FlexiblePoint) {
+  verify(message: Bytes, publicKey: FlexiblePoint): Bool {
     let msgHashBytes = Keccak.ethereum(message);
     let msgHash = keccakOutputToScalar(msgHashBytes, this.Constructor.Curve);
     return this.verifySignedHash(msgHash, publicKey);
@@ -109,7 +115,7 @@ class EcdsaSignature {
    * isValid.assertTrue('signature verifies');
    * ```
    */
-  verifyV2(message: Bytes, publicKey: FlexiblePoint) {
+  verifyV2(message: Bytes, publicKey: FlexiblePoint): Bool {
     let msgHashBytes = Keccak.ethereum(message);
     let msgHash = keccakOutputToScalar(msgHashBytes, this.Constructor.Curve);
     return this.verifySignedHashV2(msgHash, publicKey);
@@ -121,7 +127,7 @@ class EcdsaSignature {
   verifySignedHash(
     msgHash: AlmostForeignField | bigint,
     publicKey: FlexiblePoint
-  ) {
+  ): Bool {
     let msgHash_ = this.Constructor.Curve.Scalar.from(msgHash);
     let publicKey_ = this.Constructor.Curve.from(publicKey);
     return Ecdsa.verify(
@@ -142,7 +148,7 @@ class EcdsaSignature {
   verifySignedHashV2(
     msgHash: AlmostForeignField | bigint,
     publicKey: FlexiblePoint
-  ) {
+  ): Bool {
     let msgHash_ = this.Constructor.Curve.Scalar.from(msgHash);
     let publicKey_ = this.Constructor.Curve.from(publicKey);
     return Ecdsa.verifyV2(
@@ -210,8 +216,24 @@ class EcdsaSignature {
   }
 }
 
+class EcdsaSignatureV2 extends EcdsaSignature {
+  constructor(signature: {
+    r: AlmostForeignField | Field3 | bigint | number;
+    s: AlmostForeignField | Field3 | bigint | number;
+  }) {
+    super(signature);
+  }
+
+  static check(signature: EcdsaSignatureV2) {
+    multiRangeCheck(signature.r.value);
+    multiRangeCheck(signature.s.value);
+    // more efficient than the automatic check, which would do this for each scalar separately
+    this.Curve.Scalar.assertAlmostReduced(signature.r, signature.s);
+  }
+}
+
 /**
- * Create a class {@link EcdsaSignature} for verifying ECDSA signatures on the given curve.
+ * @deprecated `EcdsaSignature` is now deprecated and will be removed in a future release. Please use {@link EcdsaSignatureV2} instead.
  */
 function createEcdsa(
   curve: CurveParams | typeof ForeignCurve
@@ -231,6 +253,27 @@ function createEcdsa(
   return Signature;
 }
 
+/**
+ * Create a class {@link EcdsaSignatureV2} for verifying ECDSA signatures on the given curve.
+ */
+function createEcdsaV2(
+  curve: CurveParams | typeof ForeignCurveV2
+): typeof EcdsaSignatureV2 {
+  let Curve0: typeof ForeignCurveV2 =
+    'b' in curve ? createForeignCurveV2(curve) : curve;
+  class Curve extends Curve0 {}
+
+  class Signature extends EcdsaSignatureV2 {
+    static _Curve = Curve;
+    static _provable = provableFromClass(Signature, {
+      r: Curve.Scalar.provable,
+      s: Curve.Scalar.provable,
+    });
+  }
+
+  return Signature;
+}
+
 function toObject(signature: EcdsaSignature) {
   return { r: signature.r.value, s: signature.s.value };
 }

src/lib/provable/test/foreign-curve.unit-test.ts

@@ -1,11 +1,11 @@
-import { createForeignCurve } from '../crypto/foreign-curve.js';
+import { createForeignCurveV2 } from '../crypto/foreign-curve.js';
 import { Fq } from '../../../bindings/crypto/finite-field.js';
 import { Vesta as V } from '../../../bindings/crypto/elliptic-curve.js';
 import { Provable } from '../provable.js';
 import { Field } from '../field.js';
 import { Crypto } from '../crypto/crypto.js';
 
-class Vesta extends createForeignCurve(Crypto.CurveParams.Vesta) {}
+class Vesta extends createForeignCurveV2(Crypto.CurveParams.Vesta) {}
 class Fp extends Vesta.Scalar {}
 
 let g = { x: Fq.negate(1n), y: 2n, infinity: false };

tests/vk-regression/diverse-zk-program.ts

@@ -1,8 +1,8 @@
 import {
   ZkProgram,
   Crypto,
-  createEcdsa,
-  createForeignCurve,
+  createEcdsaV2,
+  createForeignCurveV2,
   Bytes,
   assert,
   Provable,
@@ -18,9 +18,9 @@ import {
 
 export { diverse, Bytes128 };
 
-class Secp256k1 extends createForeignCurve(Crypto.CurveParams.Secp256k1) {}
+class Secp256k1 extends createForeignCurveV2(Crypto.CurveParams.Secp256k1) {}
 class Secp256k1Scalar extends Secp256k1.Scalar {}
-class Secp256k1Signature extends createEcdsa(Secp256k1) {}
+class Secp256k1Signature extends createEcdsaV2(Secp256k1) {}
 
 class Bytes128 extends Bytes(128) {}