[Fix] properly check the upper bound for DSA signatures

roadicing · ljharb · ljharb · commit 85994cd6348b · 2023-10-21T13:08:48.000+08:00
Co-authored-by: roadicing &lt;roadicing@gmail.com&gt;
Co-authored-by: Jordan Harband &lt;ljharb@gmail.com&gt;
diff --git a/browser/verify.js b/browser/verify.js
@@ -80,7 +80,7 @@ function dsaVerify(sig, hash, pub) {
 
 function checkValue(b, q) {
   if (b.cmpn(0) <= 0) { throw new Error('invalid sig'); }
-  if (b.cmp(q) >= q) { throw new Error('invalid sig'); }
+  if (b.cmp(q) >= 0) { throw new Error('invalid sig'); }
 }
 
 module.exports = verify;
diff --git a/test/index.js b/test/index.js
@@ -5,6 +5,9 @@ var asn1 = require('parse-asn1/asn1');
 var test = require('tape').test;
 var nCrypto = require('crypto');
 var semver = require('semver');
+var BN = require('bn.js');
+var parseKeys = require('parse-asn1');
+
 var bCrypto = require('../browser');
 var fixtures = require('./fixtures');
 
@@ -154,6 +157,35 @@ fixtures.valid.ec.forEach(function (f) {
       t.end();
     });
   }
+
+  var s = parseKeys(pub).data.q;
+  test(
+    f.message + ' against a fake signature',
+    { skip: !s || '(this test only applies to DSA signatures and not EC signatures, this is ' + f.scheme + ')' },
+    function (t) {
+      var messageBase64 = Buffer.from(f.message, 'base64');
+
+      // forge a fake signature
+      var r = new BN('1');
+
+      try {
+        var fakeSig = asn1.signature.encode({ r: r, s: s }, 'der');
+      } catch (e) {
+        t.ifError(e);
+        t.end();
+        return;
+      }
+
+      var bVer = bCrypto.createVerify(f.scheme);
+      t['throws'](
+        function () { bVer.update(messageBase64).verify(pub, fakeSig); },
+        Error,
+        'fake signature is invalid'
+      );
+
+      t.end();
+    }
+  );
 });
 
 fixtures.valid.kvectors.forEach(function (f) {
