Fix Signature#normalizeS().

paulmillr · Nov 20, 2024 · b9357bb · b9357bb
1 parent 838f24a
commit b9357bb
Show file tree

Hide file tree

Showing 4 changed files with 45 additions and 5 deletions.
diff --git a/index.js b/index.js
@@ -245,7 +245,7 @@ class Signature {
     }
     hasHighS() { return high(this.s); }
     normalizeS() {
-        return high(this.s) ? new Signature(this.r, M(this.s, N), this.recovery) : this;
+        return high(this.s) ? new Signature(this.r, M(-this.s, N), this.recovery) : this;
     }
     recoverPublicKey(msgh) {
         const { r, s, recovery: rec } = this; // secg.org/sec1-v2.pdf 4.1.6

diff --git a/index.ts b/index.ts
@@ -202,7 +202,7 @@ class Signature {                                       // ECDSA Signature class
   }
   hasHighS() { return high(this.s); }
   normalizeS() {
-    return high(this.s) ? new Signature(this.r, M(this.s, N), this.recovery) : this
+    return high(this.s) ? new Signature(this.r, M(-this.s, N), this.recovery) : this;
   }
   recoverPublicKey(msgh: Hex): Point {                  // ECDSA public key recovery
     const { r, s, recovery: rec } = this;               // secg.org/sec1-v2.pdf 4.1.6

diff --git a/test/basic.test.js b/test/basic.test.js
@@ -341,9 +341,15 @@ describe(name, () => {
         fc.property(fc.hexaString({ minLength: 64, maxLength: 64 }), (msg) => {
           const priv = C.utils.randomPrivateKey();
           const pub = C.getPublicKey(priv);
-          const sig = C.sign(msg, priv);
-          const sig2 = sig.normalizeS();
-          deepStrictEqual(sig2.hasHighS(), false);
+          const sig = C.sign(msg, priv, { lowS: false });
+          if (!sig.hasHighS()) return;
+          const sigNorm = sig.normalizeS();
+          deepStrictEqual(sigNorm.hasHighS(), false, 'a');
+
+          deepStrictEqual(C.verify(sig, msg, pub, { lowS: false }), true, 'b');
+          deepStrictEqual(C.verify(sig, msg, pub, { lowS: true }), false, 'c');
+          deepStrictEqual(C.verify(sigNorm, msg, pub, { lowS: true }), true, 'd');
+          deepStrictEqual(C.verify(sigNorm, msg, pub, { lowS: false }), true, 'e');
         }),
         { numRuns: NUM_RUNS }
       )

diff --git a/test/secp256k1.test.js b/test/secp256k1.test.js
@@ -199,6 +199,40 @@ describe('secp256k1', () => {
         })
       );
     });
+
+    should('.hasHighS(), .normalizeS()', () => {
+      const priv = 'c509ae2138ddca15f6b33062cd3bf76351c79f58c82ee2c2236d835bdea19d13';
+      const msg = 'b94d27b9934d3e08a52e52d7da7dabfac484efe37a5380ee9088f7ace2efcde9';
+
+      const hi = 'a6bf36d52da4eef85a513a88d81996a47804a2390c9910c0bd35488effca36bf8bf1f9232ab0efe4a93704ae871aa953b34d1000cef59c9d33fcc696f935108d';
+      const lo = 'a6bf36d52da4eef85a513a88d81996a47804a2390c9910c0bd35488effca36bf740e06dcd54f101b56c8fb5178e556ab0761cce5e053039e8bd597f5d70130b4';
+      const hi_ = new secp.Signature(
+        75421779095773161492578598757717572512754773103551662129966816753283695785663n,
+        63299015578620006752099543153250095157058828301739590985992016204296254460045n,
+        0
+      );
+      const lo_ = new secp.Signature(
+        75421779095773161492578598757717572512754773103551662129966816753283695785663n,
+        52493073658696188671471441855437812695778735977335313396613146937221907034292n,
+        0
+      );
+
+      const pub = secp.getPublicKey(priv);
+      const sig = secp.sign(msg, priv, { lowS: false });
+      deepStrictEqual(sig.hasHighS(), true);
+      deepStrictEqual(sig, hi_);
+      deepStrictEqual(sig.toCompactHex(), hi);
+
+      const lowSig = sig.normalizeS();
+      deepStrictEqual(lowSig.hasHighS(), false);
+      deepStrictEqual(lowSig, lo_);
+      deepStrictEqual(lowSig.toCompactHex(), lo);
+
+      deepStrictEqual(secp.verify(sig, msg, pub, { lowS: false }), true);
+      deepStrictEqual(secp.verify(sig, msg, pub, { lowS: true }), false);
+      deepStrictEqual(secp.verify(lowSig, msg, pub, { lowS: true }), true);
+      deepStrictEqual(secp.verify(lowSig, msg, pub, { lowS: false }), true);
+    });
   });
 
   describe('sign()', () => {