diff --git a/p2p/p2p-util.js b/p2p/p2p-util.js index c11749118..53cf1cfef 100644 --- a/p2p/p2p-util.js +++ b/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; + } } } diff --git a/test/unit/p2p-util.test.js b/test/unit/p2p-util.test.js index 4b4a2efe7..e1454d0d1 100644 --- a/test/unit/p2p-util.test.js +++ b/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", () => { diff --git a/test/unit/transaction.test.js b/test/unit/transaction.test.js index 3ba7285c8..00c3404ee 100644 --- a/test/unit/transaction.test.js +++ b/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 + }); }); }); diff --git a/tx-pool/transaction.js b/tx-pool/transaction.js index c1dbaef8b..05324d845 100644 --- a/tx-pool/transaction.js +++ b/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);