Let exceptions from ecVerifySig() handled properly

p2p/p2p-util.js

@@ -92,12 +92,18 @@ class P2pUtil {
  }
  }
 
- static verifySignedMessage(message, address) {
+ static verifySignedMessage(message, address, chainId) {
+ const LOG_HEADER = 'verifySignedMessage';
  if (!P2pUtil._isValidMessage(message)) {
- return null;
+ return false;
  } else {
- const chainId = DB.getBlockchainParam('genesis/chain_id');
- return ainUtil.ecVerifySig(JSON.stringify(message.data.body), message.data.signature, address, chainId);
+ const cId = chainId !== undefined ? chainId : DB.getBlockchainParam('genesis/chain_id');
+ try {
+ return ainUtil.ecVerifySig(JSON.stringify(message.data.body), message.data.signature, address, cId);
+ } catch (err) {
+ logger.error(`[${LOG_HEADER}] The message is not correctly signed. Discard the message!!`);
+ return false;
+ }
  }
  }
 

test/unit/p2p-util.test.js

@@ -5,6 +5,7 @@ const expect = chai.expect;
 const assert = chai.assert;
 const { BlockchainConsts, BlockchainParams } = require('../../common/constants');
 
+// NOTE(platfowner): Run this test with AirPlay Receiver off on MacOs to avoid port number (5000) conflicts (see https://developer.apple.com/forums/thread/682332).
 describe("P2P Util", () => {
  const mockAddress = '0x012345678abcdef';
  let webServer;
@@ -240,7 +241,8 @@ describe("P2P Util", () => {
  }
  };
  const signature = util.signMessage(body, mockPrivateKey);
- it("returns null with wrong messages", () => {
+
+ it("returns false with wrong messages", () => {
  const wrongMessage1 = {
  data: {
  signature: signature
@@ -281,13 +283,13 @@ describe("P2P Util", () => {
  body: body
  }
  };
- expect(util.verifySignedMessage(wrongMessage1)).to.equal(null);
- expect(util.verifySignedMessage(wrongMessage2)).to.equal(null);
- expect(util.verifySignedMessage(wrongMessage3)).to.equal(null);
- expect(util.verifySignedMessage(wrongMessage4)).to.equal(null);
- expect(util.verifySignedMessage(wrongMessage5)).to.equal(null);
- expect(util.verifySignedMessage(wrongMessage6)).to.equal(null);
- expect(util.verifySignedMessage(wrongMessage7)).to.equal(null);
+ expect(util.verifySignedMessage(wrongMessage1)).to.equal(false);
+ expect(util.verifySignedMessage(wrongMessage2)).to.equal(false);
+ expect(util.verifySignedMessage(wrongMessage3)).to.equal(false);
+ expect(util.verifySignedMessage(wrongMessage4)).to.equal(false);
+ expect(util.verifySignedMessage(wrongMessage5)).to.equal(false);
+ expect(util.verifySignedMessage(wrongMessage6)).to.equal(false);
+ expect(util.verifySignedMessage(wrongMessage7)).to.equal(false);
  });
 
  it("verifies signature correctly", () => {
@@ -301,6 +303,18 @@ describe("P2P Util", () => {
  const address = util.getAddressFromMessage(mockMessage);
  expect(util.verifySignedMessage(mockMessage, address)).to.equal(true);
  });
+
+ it("returns false with wrong chainId", () => {
+ const mockMessage = {
+ type: 'test',
+ data: {
+ body: body,
+ signature: signature
+ }
+ };
+ const address = util.getAddressFromMessage(mockMessage);
+ expect(util.verifySignedMessage(mockMessage, address, 1)).to.equal(false); // with wrong chainId = 1
+ });
  });
 
  describe("toHostname", () => {

test/unit/transaction.test.js

@@ -354,5 +354,10 @@ describe('Transaction', () => {
  txParentHash.tx_body.billing = 'app_b|0';
  expect(Transaction.verifyTransaction(txParentHash)).to.equal(false);
  });
+
+ it('fail to verify an invalid transaction with a wrong chainId', () => {
+ txParentHash.tx_body.billing = 'app_b|0';
+ expect(Transaction.verifyTransaction(tx, 1)).to.equal(false); // with a wrong chainId = 1
+ });
  });
 });

tx-pool/transaction.js

@@ -181,36 +181,40 @@ class Transaction {
  }
 
  static verifyTransaction(tx, chainId) {
+ const LOG_HEADER = 'verifyTransaction';
  if (!tx || !Transaction.isValidTxBody(tx.tx_body)) {
- logger.info(`Invalid transaction body: ${JSON.stringify(tx, null, 2)}`);
+ logger.info(`[${LOG_HEADER}] Invalid transaction body: ${JSON.stringify(tx, null, 2)}`);
  return false;
  }
  // A devel method for bypassing the transaction verification.
  if (_.get(tx, 'extra.skip_verif')) {
- logger.info('Skip verifying signature for transaction: ' + JSON.stringify(tx, null, 2));
+ logger.info(`[${LOG_HEADER}] Skip verifying signature for transaction: ` + JSON.stringify(tx, null, 2));
  return true;
  }
- return ainUtil.ecVerifySig(tx.tx_body, tx.signature, tx.address, chainId);
+ try {
+ return ainUtil.ecVerifySig(tx.tx_body, tx.signature, tx.address, chainId);
+ } catch (err) {
+ logger.info(`[${LOG_HEADER}] Signature verifycation failed with error: ${err.message}`);
+ return false;
+ }
  }
 
  static isValidTxBody(txBody) {
+ const LOG_HEADER = 'isValidTxBody';
  if (!Transaction.hasRequiredFields(txBody)) {
- logger.info(`Transaction body has some missing fields: ${JSON.stringify(txBody, null, 2)}`);
+ logger.info(`[${LOG_HEADER}] Transaction body has some missing fields: ${JSON.stringify(txBody, null, 2)}`);
  return false;
  }
  if (!Transaction.isValidNonce(txBody.nonce)) {
- logger.info(
- `Transaction body has invalid nonce: ${JSON.stringify(txBody, null, 2)}`);
+ logger.info(`[${LOG_HEADER}] Transaction body has invalid nonce: ${JSON.stringify(txBody, null, 2)}`);
  return false;
  }
  if (!Transaction.isValidGasPrice(txBody.gas_price)) {
- logger.info(
- `Transaction body has invalid gas price: ${JSON.stringify(txBody, null, 2)}`);
+ logger.info(`[${LOG_HEADER}] Transaction body has invalid gas price: ${JSON.stringify(txBody, null, 2)}`);
  return false;
  }
  if (!Transaction.isValidBilling(txBody.billing)) {
- logger.info(
- `Transaction body has invalid billing: ${JSON.stringify(txBody, null, 2)}`);
+ logger.info(`[${LOG_HEADER}] Transaction body has invalid billing: ${JSON.stringify(txBody, null, 2)}`);
  return false;
  }
  return Transaction.isInStandardFormat(txBody);