From f8ea7e39cc31eecec69b198f2076ac09c6689581 Mon Sep 17 00:00:00 2001 From: Braydon Fuller Date: Thu, 30 Apr 2015 13:14:19 -0400 Subject: [PATCH] Included satoshis check during checked serialization. --- lib/errors/spec.js | 3 +++ lib/transaction/output.js | 15 +++++++++++++++ lib/transaction/transaction.js | 32 ++++++++++++++++++++------------ test/transaction/transaction.js | 16 ++++++++++++++-- 4 files changed, 52 insertions(+), 14 deletions(-) diff --git a/lib/errors/spec.js b/lib/errors/spec.js index 3815b59..81a3e2f 100644 --- a/lib/errors/spec.js +++ b/lib/errors/spec.js @@ -81,6 +81,9 @@ module.exports = [{ }, { name: 'DustOutputs', message: 'Dust amount detected in one output' + }, { + name: 'InvalidSatoshis', + message: 'Output satoshis are invalid', }, { name: 'FeeError', message: 'Fees are not correctly set {0}', diff --git a/lib/transaction/output.js b/lib/transaction/output.js index 20823f3..558f38e 100644 --- a/lib/transaction/output.js +++ b/lib/transaction/output.js @@ -9,6 +9,8 @@ var BufferWriter = require('../encoding/bufferwriter'); var Script = require('../script'); var $ = require('../util/preconditions'); +var MAX_SAFE_INTEGER = 0x1fffffffffffff; + function Output(params) { if (!(this instanceof Output)) { return new Output(params); @@ -62,6 +64,19 @@ Object.defineProperty(Output.prototype, 'satoshis', { } }); +Output.prototype.invalidSatoshis = function() { + if (this._satoshis > MAX_SAFE_INTEGER) { + return 'transaction txout satoshis greater than max safe integer'; + } + if (this._satoshis !== this._satoshisBN.toNumber()) { + return 'transaction txout satoshis has corrupted value'; + } + if (this._satoshis < 0) { + return 'transaction txout negative'; + } + return false; +}; + Output.prototype._fromObject = function(param) { this.satoshis = param.satoshis; if (param.script || param.scriptBuffer) { diff --git a/lib/transaction/transaction.js b/lib/transaction/transaction.js index ac8acdb..8749355 100644 --- a/lib/transaction/transaction.js +++ b/lib/transaction/transaction.js @@ -23,8 +23,6 @@ var Script = require('../script'); var PrivateKey = require('../privatekey'); var BN = require('../crypto/bn'); -var MAX_BLOCK_SIZE = 1000000; - /** * Represents a transaction, a set of inputs and outputs to change ownership of tokens * @@ -61,7 +59,7 @@ function Transaction(serialized) { var CURRENT_VERSION = 1; var DEFAULT_NLOCKTIME = 0; -var MAX_SAFE_INTEGER = 0x1fffffffffffff; +var MAX_BLOCK_SIZE = 1000000; // Minimum amount for an output for it not to be considered a dust output Transaction.DUST_AMOUNT = 546; @@ -169,12 +167,22 @@ Transaction.prototype.checkedSerialize = function(opts) { var serializationError = this.getSerializationError(opts); if (serializationError) { serializationError.message += ' Use Transaction#uncheckedSerialize if you want to skip security checks. ' + - 'See http://bitcore.io/guide/transaction.html#Serialization for more info.' + 'See http://bitcore.io/guide/transaction.html#Serialization for more info.'; throw serializationError; } return this.uncheckedSerialize(); }; +Transaction.prototype.invalidSatoshis = function() { + var invalid = false; + for (var i = 0; i < this.outputs.length; i++) { + if (this.outputs[i].invalidSatoshis()) { + invalid = true; + } + } + return invalid; +}; + /** * Retrieve a possible error that could appear when trying to serialize and broadcast this transaction * @@ -183,6 +191,11 @@ Transaction.prototype.checkedSerialize = function(opts) { */ Transaction.prototype.getSerializationError = function(opts) { opts = opts || {}; + + if (this.invalidSatoshis()) { + return new errors.Transaction.InvalidSatoshis(); + } + var missingChange = this._missingChange(); var feeIsTooLarge = this._isFeeTooLarge(); var feeIsTooSmall = this._isFeeTooSmall(); @@ -986,14 +999,9 @@ Transaction.prototype.verify = function() { var valueoutbn = new BN(0); for (var i = 0; i < this.outputs.length; i++) { var txout = this.outputs[i]; - if (txout._satoshis > MAX_SAFE_INTEGER) { - return 'transaction txout ' + i + ' satoshis greater than max safe integer'; - } - if (txout._satoshis !== txout._satoshisBN.toNumber()) { - return 'transaction txout ' + i + ' satoshis has corrupted value'; - } - if (txout._satoshis < 0) { - return 'transaction txout ' + i + ' negative'; + + if (txout.invalidSatoshis()) { + return 'transaction txout ' + i + ' satoshis is invalid'; } if (txout._satoshisBN.gt(new BN(Transaction.MAX_MONEY, 10))) { return 'transaction txout ' + i + ' greater than MAX_MONEY'; diff --git a/test/transaction/transaction.js b/test/transaction/transaction.js index 27f8b5f..7841f8a 100644 --- a/test/transaction/transaction.js +++ b/test/transaction/transaction.js @@ -246,6 +246,18 @@ describe('Transaction', function() { transaction.outputs.length.should.equal(2); transaction.outputs[1].satoshis.should.equal(10000); }); + it('if satoshis are invalid', function() { + var transaction = new Transaction() + .from(simpleUtxoWith100000Satoshis) + .to(toAddress, 99999) + .change(changeAddress) + .sign(privateKey); + transaction.outputs[0]._satoshis = 100; + transaction.outputs[0]._satoshisBN = new BN(101, 10); + expect(function() { + return transaction.serialize(); + }).to.throw(errors.Transaction.InvalidSatoshis); + }); it('if fee is too small, fail serialization', function() { var transaction = new Transaction() .from(simpleUtxoWith100000Satoshis) @@ -425,7 +437,7 @@ describe('Transaction', function() { tx.outputs[0]._satoshis = 100; tx.outputs[0]._satoshisBN = new BN('fffffffffffffff', 16); var verify = tx.verify(); - verify.should.equal('transaction txout 0 satoshis has corrupted value'); + verify.should.equal('transaction txout 0 satoshis is invalid'); }); it('not if _satoshis is negative', function() { @@ -440,7 +452,7 @@ describe('Transaction', function() { tx.outputs[0]._satoshis = -100; tx.outputs[0]._satoshisBN = new BN(-100, 10); var verify = tx.verify(); - verify.should.equal('transaction txout 0 negative'); + verify.should.equal('transaction txout 0 satoshis is invalid'); }); it('not if transaction is greater than max block size', function() {