fix: instantlock#verify crashing if bls lib can't parse the signature (…

…#211)

lib/chainlock/chainlock.js

@@ -146,20 +146,11 @@ class ChainLock {
    * @returns {Promise<Boolean>} - returns the result of the signature verification
    */
   async verifySignatureAgainstQuorum(quorumEntry, requestId) {
-    const { signature } = this;
-    const { quorumPublicKey } = quorumEntry;
-    const signHash = this.getSignHashForQuorumEntry(quorumEntry, requestId);
-
-    const blsInstance = await bls.getInstance();
-
-    const quorumPubKey = blsInstance.PublicKey.fromBytes(Buffer.from(quorumPublicKey, 'hex'));
-
-    const aggregationInfo = blsInstance.AggregationInfo.fromMsgHash(quorumPubKey, signHash);
-    const thresholdSignature = blsInstance.Signature.fromBytes(Buffer.from(signature, 'hex'));
-
-    thresholdSignature.setAggregationInfo(aggregationInfo);
-
-    return thresholdSignature.verify();
+    return bls.verifySignature(
+      this.signature.toString('hex'),
+      this.getSignHashForQuorumEntry(quorumEntry, requestId),
+      quorumEntry.quorumPublicKey,
+    );
   }
 
   /**

lib/crypto/bls.js

@@ -32,6 +32,43 @@ const bls = {
       }
     });
   },
+  /**
+   * Validate bls signature
+   * @param {string} signatureHex
+   * @param {Uint8Array} messageHash
+   * @param {string} publicKeyHex
+   * @return {Promise<boolean>}
+   */
+  async verifySignature(signatureHex, messageHash, publicKeyHex) {
+    const blsInstance = await this.getInstance();
+    let result = false;
+
+    let thresholdSignature;
+    let quorumPubKey;
+    let aggregationInfo;
+
+    try {
+      thresholdSignature = blsInstance.Signature.fromBytes(Uint8Array.from(Buffer.from(signatureHex, 'hex')));
+      quorumPubKey = blsInstance.PublicKey.fromBytes(Uint8Array.from(Buffer.from(publicKeyHex, 'hex')));
+      aggregationInfo = blsInstance.AggregationInfo.fromMsgHash(quorumPubKey, messageHash);
+
+      thresholdSignature.setAggregationInfo(aggregationInfo);
+
+      result = thresholdSignature.verify();
+    } catch (e) {
+      // This line is because BLS is a c++ WebAssembly binding, it will throw
+      // cryptic error messages if it fails to parse the signature.
+      return result;
+    } finally {
+      // Values from emscripten compiled code can't be garbage collected in JS,
+      // so they have to be released first using .delete method
+      if (thresholdSignature) { thresholdSignature.delete(); }
+      if (quorumPubKey) { quorumPubKey.delete(); }
+      if (aggregationInfo) { aggregationInfo.delete(); }
+    }
+
+    return result;
+  },
 };
 
 module.exports = bls;

lib/deterministicmnlist/QuorumEntry.js

@@ -300,27 +300,16 @@ QuorumEntry.prototype.getCommitmentHash = function getCommitmentHash() {
  * Verifies the quorum's bls threshold signature
  * @return {Promise<boolean>}
  */
-QuorumEntry.prototype.isValidQuorumSig = function isValidQuorumSig() {
-  return new Promise((resolve, reject) => {
-    if (this.isOutdatedRPC) {
-      return reject(new Error('Quorum cannot be verified: node running on outdated DashCore version (< 0.16)'));
-    }
-
-    return bls.getInstance().then((blsInstance) => {
-      const quorumPubKey = blsInstance.PublicKey.fromBytes(Uint8Array.from(Buffer.from(this.quorumPublicKey, 'hex')));
-      const msgHash = Uint8Array.from(this.getCommitmentHash());
-      const aggregationInfo = blsInstance.AggregationInfo.fromMsgHash(quorumPubKey, msgHash);
-      const thresholdSignature = blsInstance.Signature.fromBytes(Uint8Array.from(Buffer.from(this.quorumSig, 'hex')));
-
-      thresholdSignature.setAggregationInfo(aggregationInfo);
+QuorumEntry.prototype.isValidQuorumSig = async function isValidQuorumSig() {
+  if (this.isOutdatedRPC) {
+    throw new Error('Quorum cannot be verified: node running on outdated DashCore version (< 0.16)');
+  }
 
-      const result = thresholdSignature.verify();
-      quorumPubKey.delete();
-      thresholdSignature.delete();
-      aggregationInfo.delete();
-      resolve(result);
-    });
-  });
+  return bls.verifySignature(
+    this.quorumSig,
+    Uint8Array.from(this.getCommitmentHash()),
+    this.quorumPublicKey,
+  );
 };
 
 /**

lib/instantlock/instantlock.js

@@ -147,20 +147,11 @@ class InstantLock {
    * @returns {Promise<Boolean>} - returns the result of the signature verification
    */
   async verifySignatureAgainstQuorum(quorumEntry, requestId) {
-    const { signature } = this;
-    const { quorumPublicKey } = quorumEntry;
-    const signHash = this.getSignHashForQuorumEntry(quorumEntry, requestId);
-
-    const blsInstance = await bls.getInstance();
-
-    const quorumPubKey = blsInstance.PublicKey.fromBytes(Buffer.from(quorumPublicKey, 'hex'));
-
-    const aggregationInfo = blsInstance.AggregationInfo.fromMsgHash(quorumPubKey, signHash);
-    const thresholdSignature = blsInstance.Signature.fromBytes(Buffer.from(signature, 'hex'));
-
-    thresholdSignature.setAggregationInfo(aggregationInfo);
-
-    return thresholdSignature.verify();
+    return bls.verifySignature(
+      this.signature,
+      this.getSignHashForQuorumEntry(quorumEntry, requestId),
+      quorumEntry.quorumPublicKey,
+    );
   }
 
   /**

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@dashevo/dashcore-lib",
-  "version": "0.19.15",
+  "version": "0.19.16",
   "description": "A pure and powerful JavaScript Dash library.",
   "author": "Dash Core Group, Inc. <[email protected]>",
   "main": "index.js",

test/chainlock/chainlock.js

@@ -1,7 +1,6 @@
 'use strict';
 
 const chai = require('chai');
-const should = chai.should();
 const expect = chai.expect;
 
 const bitcore = require('../../index');

test/deterministicmnlist/QuorumEntry.js

@@ -40,6 +40,7 @@ var commitmentHash = "bc8c5dc5975ce6c4988ce8506ce6a4ec59b3232a8715aa2ffaeeebab8d
 var selectionModifier = "c6a87d306a29918722342ddd612262356097b50b3d67476f073c33947aee32f0";
 
 describe('QuorumEntry', function () {
+  this.timeout(10000);
   describe('fromBuffer', function () {
     it('Should be able to parse data from a buffer', function () {
       var entry = QuorumEntry.fromBuffer(Buffer.from(quorumEntryHex, 'hex'));

test/deterministicmnlist/SimplifiedMNList.js

@@ -8,7 +8,7 @@ var constants = require('../../lib/constants');
 var Networks = require('../../lib/networks');
 
 describe('SimplifiedMNList', function () {
-  this.timeout(5000);
+  this.timeout(10000);
 
   describe('constructor', function () {
     it('Should call applyDiff with the first argument passed to the constructor', function () {

test/instantlock/instantlock.js

@@ -11,6 +11,7 @@ const InstantLock = bitcore.InstantLock;
 const QuorumEntry = bitcore.QuorumEntry;
 
 describe('InstantLock', function () {
+  this.timeout(10000);
   let object;
   let str;
   let buf;
@@ -137,14 +138,22 @@ describe('InstantLock', function () {
       });
     });
     describe('#verify', function () {
-      this.timeout(6000);
       it('should verify signature against SMLStore', async function () {
         const instantLock = new InstantLock(buf2);
         const smlDiffArray = SMNListFixture.getInstantLockDiffArray();
         const SMLStore = new SimplifiedMNListStore(smlDiffArray);
         const isValid = await instantLock.verify(SMLStore);
         expect(isValid).to.equal(true);
       });
+      it('should not crash if BLS fails to parse the signature or any other data', async function () {
+        const instantLock = new InstantLock(buf2);
+        expect(instantLock.signature.length).to.be.equal(192);
+        instantLock.signature = '0'.repeat(192);
+        const smlDiffArray = SMNListFixture.getInstantLockDiffArray();
+        const SMLStore = new SimplifiedMNListStore(smlDiffArray);
+        const isValid = await instantLock.verify(SMLStore);
+        expect(isValid).to.equal(false);
+      })
     });
   });