From 9011fcc25e0e5e162c77ee195cb97bd374760965 Mon Sep 17 00:00:00 2001 From: Christopher Jeffrey Date: Mon, 18 Jan 2016 01:52:18 -0800 Subject: [PATCH] stricter script standards. work on tx. --- lib/bcoin/block.js | 4 +- lib/bcoin/input.js | 35 ++--- lib/bcoin/output.js | 10 +- lib/bcoin/script.js | 325 +++++++++++++++++++++++++++++++------------- lib/bcoin/tx.js | 43 ++++-- test/wallet-test.js | 3 +- 6 files changed, 287 insertions(+), 133 deletions(-) diff --git a/lib/bcoin/block.js b/lib/bcoin/block.js index 2ab90cd0..d6626ffe 100644 --- a/lib/bcoin/block.js +++ b/lib/bcoin/block.js @@ -386,11 +386,11 @@ Block.prototype.verifyContext = function verifyContext() { // Signature validation is now enforced (bip66) if (!(this.version >= 3 && prev.isUpgraded(3))) - flags.strictder = false; + flags.dersig = false; // CHECKLOCKTIMEVERIFY is now usable (bip65) if (!(this.version >= 4 && prev.isUpgraded(4))) - flags.cltv = false; + flags.checklocktimeverify = false; // Use nLockTime median past (bip113) // https://github.com/btcdrak/bips/blob/d4c9a236ecb947866c61aefb868b284498489c2b/bip-0113.mediawiki diff --git a/lib/bcoin/input.js b/lib/bcoin/input.js index 3095d51f..3ea0f49a 100644 --- a/lib/bcoin/input.js +++ b/lib/bcoin/input.js @@ -14,41 +14,23 @@ var constants = bcoin.protocol.constants; */ function Input(options) { - var tx = options.tx; - var lock; - if (!(this instanceof Input)) return new Input(options); - if (!tx) - throw new Error('No TX passed into Input.'); - this.out = { tx: options.out.tx || null, - hash: options.out.hash || null, + hash: options.out.hash, index: options.out.index }; + if (typeof this.out.hash !== 'string') + this.out.hash = utils.toHex(this.out.hash); + this.script = options.script ? options.script.slice() : []; this.seq = options.seq === undefined ? 0xffffffff : options.seq; if (options.script && options.script._raw) utils.hidden(this.script, '_raw', options.script._raw); - - if (this.output) { - lock = this.lock; - if (lock > 0) { - if (tx._lock === 0) - tx.lock = Math.max(lock, tx.lock); - if (!bcoin.script.spendable(this.output.script, tx.lock)) - throw new Error('Cannot spend ' + utils.revHex(this.out.hash)); - } - } - - if (tx.lock !== 0) { - if (options.seq === undefined) - this.seq = 0; - } } Input.prototype.__defineGetter__('data', function() { @@ -120,7 +102,13 @@ Input.prototype.__defineGetter__('n', function() { Input.prototype.__defineGetter__('lock', function() { if (!this.output) return 0; - return this.output.lock || 0; + return this.output.lock; +}); + +Input.prototype.__defineGetter__('lockType', function() { + if (!this.output) + return 'height'; + return this.output.lockType; }); Input.prototype.__defineGetter__('text', function() { @@ -264,6 +252,7 @@ Input.prototype.inspect = function inspect() { keys: this.keys.map(utils.toHex), text: this.text, lock: this.lock, + lockType: this.lockType, value: utils.btc(output.value), script: bcoin.script.format(this.script)[0], redeem: this.redeem ? bcoin.script.format(this.redeem)[0] : null, diff --git a/lib/bcoin/output.js b/lib/bcoin/output.js index c24eba9c..5da44e34 100644 --- a/lib/bcoin/output.js +++ b/lib/bcoin/output.js @@ -8,21 +8,18 @@ var bn = require('bn.js'); var bcoin = require('../bcoin'); var utils = bcoin.utils; var assert = utils.assert; +var constants = bcoin.protocol.constants; /** * Output */ function Output(options) { - var tx = options.tx; var value; if (!(this instanceof Output)) return new Output(options); - if (!tx) - throw new Error('No TX passed into Output.'); - value = options.value; if (typeof value === 'number' && (value | 0) === value) @@ -108,6 +105,10 @@ Output.prototype.__defineGetter__('lock', function() { return bcoin.script.lockTime(this.script); }); +Output.prototype.__defineGetter__('lockType', function() { + return this.lock < constants.locktimeThreshold ? 'height' : 'time'; +}); + Output.prototype.__defineGetter__('text', function() { return this.data.text; }); @@ -201,6 +202,7 @@ Output.prototype.inspect = function inspect() { n: this.n, text: this.text, lock: this.lock, + lockType: this.lockType, value: utils.btc(this.value), script: bcoin.script.format(this.script)[0] }; diff --git a/lib/bcoin/script.js b/lib/bcoin/script.js index f3aeb9c4..3e5d5f05 100644 --- a/lib/bcoin/script.js +++ b/lib/bcoin/script.js @@ -201,6 +201,11 @@ script.verify = function verify(input, output, tx, i, flags) { if (!flags) flags = {}; + if (flags.sigpushonly !== false) { + if (!script.pushOnly(input)) + return false; + } + // Execute the input script script.execute(input, stack, tx, i, flags); @@ -273,14 +278,22 @@ script.subscript = function subscript(s, lastSep) { return res; }; -script.checksig = function checksig(msg, sig, pub) { +script.checksig = function checksig(msg, sig, key) { var k; - if (pub.getPublic) - pub = pub.getPublic(); + if (key.getPublic) + key = key.getPublic(); + + if (!utils.isBuffer(sig)) + return false; + + if (sig.length === 0) + return false; + + sig = sig.slice(0, -1); try { - k = bcoin.ecdsa.keyPair({ pub: pub }); + k = bcoin.ecdsa.keyPair({ pub: key }); } catch (e) { return false; } @@ -294,12 +307,37 @@ script.checksig = function checksig(msg, sig, pub) { // Use a try catch in case there are // any uncaught errors for bad inputs in verify(). try { - return bcoin.ecdsa.verify(msg, sig, pub); + return bcoin.ecdsa.verify(msg, sig, key); } catch (e) { return false; } }; +script.sign = function sign(msg, key, type) { + var half = bcoin.ecdsa.n.ushrn(1); + var sig = bcoin.ecdsa.sign(msg, key.priv); + + // Elliptic shouldn't be generating + // negative S values. + assert(sig.s.cmpn(0) > 0); + + // If S value is bigger than half of the + // order of the curve, it's too damn big. + // Subtract from the `n` order to make + // it smaller. + if (sig.s.cmp(half) > 0) + sig.s = bcoin.ecdsa.n.sub(sig.s); + + // Convert to DER array + sig = sig.toDER(); + + // Add the sighash type as a single byte + // to the signature. + sig = sig.concat(type); + + return sig; +}; + script._next = function _next(to, s, pc) { var depth = 0; var o; @@ -342,7 +380,7 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { var pc = 0; var o, val; var if_, else_, endif; - var v, v1, v2, v3, v4; + var v1, v2, v3, v4; var n, n1, n2, n3; var res; var key, sig, type, subscript, hash; @@ -359,6 +397,8 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { if (Array.isArray(o)) { if (o.length > constants.script.maxPush) return false; + if (!script.checkPush(o, flags)) + return false; stack.push(o); continue; } @@ -378,6 +418,8 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { case 'nop8': case 'nop9': case 'nop10': { + if (flags.discourage_nops !== false) + return false; break; } case '1negate': { @@ -388,8 +430,7 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { case 'notif': { if (stack.length < 1) return false; - v = stack.pop(); - val = script.num(v).cmpn(0) !== 0; + val = script.bool(stack.pop()); if (o === 'notif') val = !val; if_ = pc; @@ -429,7 +470,7 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { case 'verify': { if (stack.length === 0) return false; - if (script.num(stack.pop()).cmpn(0) === 0) + if (!script.bool(stack.pop())) return false; break; } @@ -451,7 +492,7 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { case 'ifdup': { if (stack.length === 0) return false; - if (script.num(stack[stack.length - 1]).cmpn(0) !== 0) + if (script.bool(stack[stack.length - 1])) stack.push(script.array(stack[stack.length - 1])); break; } @@ -487,16 +528,16 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { case 'roll': { if (stack.length < 2) return false; - v = stack.pop(); - if (v.length > 6) + val = stack.pop(); + if (val.length > 6) return false; - n = script.num(v, true); + n = script.num(val, true); if (n < 0 || n >= stack.length) return false; - v = stack[-n - 1]; + val = stack[-n - 1]; if (o === 'roll') stack.splice(stack.length - n - 1, 1); - stack.push(v); + stack.push(val); break; } case 'rot': { @@ -706,7 +747,7 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { } if (typeof n === 'boolean') n = script.num(+n); - res = n.cmpn(0) !== 0; + res = script.bool(n); if (o === 'numequalverify') { if (!res) return false; @@ -782,30 +823,20 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { key = stack.pop(); sig = stack.pop(); - if (!script.isKey(key)) + if (!script.isValidKey(key, flags)) return false; - if (!script.isSignature(sig)) + if (!script.isValidSignature(sig, flags)) return false; - if (flags.strictder !== false) { - if (!script.isValidSignature(sig)) - return false; - if (!script.isLowDER(sig.slice(0, -1))) - return false; - } - type = sig[sig.length - 1]; - if (!constants.hashTypeByVal[type & 0x1f]) - return false; - subscript = script.subscript(data, lastSep); script.removeData(subscript, sig); hash = tx.signatureHash(index, subscript, type); - res = script.checksig(hash, sig.slice(0, -1), key); + res = script.checksig(hash, sig, key); if (o === 'checksigverify') { if (!res) return false; @@ -831,7 +862,8 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { keys = []; for (i = 0; i < n; i++) { key = stack.pop(); - if (!script.isKey(key)) + + if (!script.isValidKey(key, flags)) return false; keys.push(key); @@ -856,26 +888,16 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { for (i = 0, j = 0; i < m; i++) { sig = stack.pop(); - if (!script.isSignature(sig)) + if (!script.isValidSignature(sig, flags)) return false; - if (flags.strictder !== false) { - if (!script.isValidSignature(sig)) - return false; - if (!script.isLowDER(sig.slice(0, -1))) - return false; - } - type = sig[sig.length - 1]; - if (!constants.hashTypeByVal[type & 0x1f]) - return false; - hash = tx.signatureHash(index, subscript, type); res = false; for (; !res && j < n; j++) - res = script.checksig(hash, sig.slice(0, -1), keys[j]); + res = script.checksig(hash, sig, keys[j]); if (res) succ++; @@ -903,8 +925,11 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { } case 'checklocktimeverify': { // OP_CHECKLOCKTIMEVERIFY = OP_NOP2 - if (flags.cltv === false) + if (flags.checklocktimeverify === false) { + if (flags.discourage_nops !== false) + return false; break; + } if (!tx || stack.length === 0) return false; @@ -940,8 +965,11 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { } case 'nop1': { // OP_EVAL = OP_NOP1 - if (!flags.allowEval) + if (flags['eval'] !== true) { + if (flags.discourage_nops !== false) + return false; break; + } recurse = recurse || 0; @@ -981,13 +1009,45 @@ script.execute = function execute(data, stack, tx, index, flags, recurse) { return true; }; -script.num = function num(value, useNum) { - if (utils.isFinite(value)) - return useNum ? value : new bn(value, 'le'); +script.bool = function bool(value) { + var i; + + // Should never happen: + // if (typeof value === 'boolean') + // return value; + + // Should never happen: + // if (utils.isFinite(value)) + // return value !== 0; + + if (value instanceof bn) + return value.cmpn(0) !== 0; assert(utils.isBuffer(value)); - if (script.requireminimal && value.length > 0) { + for (i = 0; i < value.length; i++) { + if (value[i] !== 0) { + // Cannot be negative zero + if (i === value.length - 1 && value[i] === 0x80) + return false; + return true; + } + } + + return false; +}; + +script.num = function num(value, useNum, minimaldata) { + if (utils.isFinite(value)) + return useNum ? value : new bn(value, 'le'); + + // Should never happen: + // if (value instanceof bn) + // return useNum ? value.toNumber() : value; + + assert(utils.isBuffer(value)); + + if (minimaldata && value.length > 0) { // If the low bits on the last byte are unset, // fail if The value's second to last byte does // not have the high bit set. A number can't @@ -1064,27 +1124,35 @@ script.removeData = function removeData(s, data) { } }; -script.checkPush = function checkPush(op, value) { - if (!script.requireminimal) +script.checkPush = function checkPush(op, value, flags) { + if (!flags) + flags = {}; + + if (flags.minimaldata === false) return true; + if (!op.pushdata) + return true; + + op = op.pushdata.opcode || op.pushdata.len; + if (value.length === 1 && value[0] === 0) - return op === constants.opcodes['0']; + return op === '0'; if (value.length === 1 && value[0] >= 1 && value[0] <= 16) - return op >= constants.opcodes['1'] && op <= constants.opcodes['16']; + return +op >= 1 && +op <= 16; if (value.length === 1 && value[0] === -1) - return op === constants.opcodes['1negate']; + return op === '1negate'; if (value.length <= 75) return op === value.length; if (value.length <= 255) - return op === constants.opcodes.pushdata1; + return op === 'pushdata1'; if (value.length <= 65535) - return op === constants.opcodes.pushdata2; + return op === 'pushdata2'; return true; }; @@ -1148,19 +1216,29 @@ script.isEncoded = function isEncoded(s) { return utils.isBytes(s); }; -script.isLockTime = function isLockTime(s, check) { - return s.length > 4 - && Array.isArray(s[0]) - && s[1] === 'checklocktimeverify' - && s[2] === 'drop' - && s[3] === 'codeseparator'; +script._lockTime = function _lockTime(s) { + var i; + + if (s.length < 2) + return; + + for (i = 0; i < s.length; i++) { + if (Array.isArray(s[i]) && s[i + 1] === 'checklocktimeverify') + return s[i]; + } +}; + +script.isLockTime = function isLockTime(s) { + return script._lockTime(s) != null; }; script.lockTime = function lockTime(s) { - if (!script.isLockTime(s)) + var lockTime = script._lockTime(s); + + if (!lockTime) return 0; - return script.num(s[0], true); + return script.num(lockTime, true); }; script.spendable = function spendable(s, lockTime) { @@ -1729,13 +1807,10 @@ script.isKey = function isKey(key) { return key.length >= 33 && key.length <= 65; }; -script.isSignature = function isSignature(sig, allowZero) { +script.isSignature = function isSignature(sig) { if (!utils.isBuffer(sig)) return false; - if (allowZero && sig.length === 0) - return true; - return sig.length >= 9 && sig.length <= 73; }; @@ -1753,6 +1828,72 @@ script.isData = function isData(data) { return data.length <= constants.script.maxOpReturn; }; +script.isValidKey = function isValidKey(key, flags) { + if (!flags) + flags = {}; + + if (!utils.isBuffer(key)) + return false; + + if (flags.strictenc !== false) { + if (!script.isKeyEncoding(key)) + return false; + } + + return true; +}; + +script.isKeyEncoding = function isKeyEncoding(key) { + if (!utils.isBuffer(key)) + return false; + + if (key.length < 33) + return false; + + if (key[0] === 0x04) { + if (key.length !== 65) + return false; + } else if (key[0] === 0x02 || key[0] === 0x03) { + if (key.length !== 33) + return false; + } else { + return false; + } + + return true; +}; + +script.isValidSignature = function isValidSignature(sig, flags) { + if (!flags) + flags = {}; + + if (!utils.isBuffer(sig)) + return false; + + // Allow empty sigs + if (sig.length === 0) + return true; + + if (flags.dersig !== false + || flags.low_s !== false + || flags.strictenc !== false) { + if (!script.isSignatureEncoding(sig)) + return false; + } + + if (flags.low_s !== false) { + if (!script.isLowDER(sig)) + return false; + } + + if (flags.strictenc !== false) { + if (!script.isHashType(sig)) + return false; + } + + return true; +}; + // https://github.com/bitcoin/bips/blob/master/bip-0066.mediawiki /** * A canonical signature exists of: <30> <02> <02> @@ -1764,17 +1905,12 @@ script.isData = function isData(data) { * * This function is consensus-critical since BIP66. */ -script.isValidSignature = function isValidSignature(sig, allowZero) { +script.isSignatureEncoding = function isSignatureEncoding(sig) { var lenR, lenS; if (!utils.isBuffer(sig)) return false; - // Empty signature. Not strictly DER encoded, but allowed to provide a - // compact way to provide an invalid signature for use with CHECK(MULTI)SIG - if (allowZero && sig.length === 0) - return true; - // Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S] [sighash] // * total-length: 1-byte length descriptor of everything that follows, // excluding the sighash byte. @@ -1790,6 +1926,7 @@ script.isValidSignature = function isValidSignature(sig, allowZero) { // Minimum and maximum size constraints. if (sig.length < 9) return false; + if (sig.length > 73) return false; @@ -1853,13 +1990,33 @@ script.isValidSignature = function isValidSignature(sig, allowZero) { return true; }; +script.isHashType = function isHashType(sig) { + if (!utils.isBuffer(sig)) + return false; + + if (sig.length === 0) + return false; + + type = sig[sig.length - 1] & ~constants.hashType.anyonecanpay; + + if (!constants.hashTypeByVal[type]) + return false; + + return true; +}; + script.isLowDER = function isLowDER(sig) { var half = bcoin.ecdsa.n.ushrn(1); if (!sig.s) { - assert(utils.isBuffer(sig)); + if (!utils.isBuffer(sig)) + return false; + + if (!script.isSignatureEncoding(sig)) + return false; + try { - sig = new bcoin.signature(sig); + sig = new bcoin.signature(sig.slice(0, -1)); } catch (e) { return false; } @@ -1879,24 +2036,6 @@ script.isLowDER = function isLowDER(sig) { return true; }; -script.sign = function sign(msg, key) { - var half = bcoin.ecdsa.n.ushrn(1); - var sig = bcoin.ecdsa.sign(msg, key.priv); - - // Elliptic shouldn't be generating - // negative S values. - assert(sig.s.cmpn(0) > 0); - - // S value is already low. - if (sig.s.cmp(half) <= 0) - return sig.toDER(); - - // Subtract from the `n` order to make it smaller. - sig.s = bcoin.ecdsa.n.sub(sig.s); - - return sig.toDER(); -}; - script.format = function format(input, output) { var scripts = []; var prev, redeem; diff --git a/lib/bcoin/tx.js b/lib/bcoin/tx.js index 608cb233..923371ec 100644 --- a/lib/bcoin/tx.js +++ b/lib/bcoin/tx.js @@ -36,20 +36,21 @@ function TX(data, block) { this.network = data.network || false; this.relayedBy = data.relayedBy || '0.0.0.0'; + this.rbf = !!data.rbf; this._chain = data.chain; - this._lock = this.lock; - if (data.inputs) { + assert(this.inputs.length === 0); data.inputs.forEach(function(input) { - this.input(input, null); + this.inputs.push(bcoin.input(input)); }, this); } if (data.outputs) { - data.outputs.forEach(function(out) { - this.out(out, null); + assert(this.outputs.length === 0); + data.outputs.forEach(function(output) { + this.outputs.push(bcoin.output(output)); }, this); } @@ -66,6 +67,13 @@ function TX(data, block) { // ps = Pending Since this.ps = this.ts === 0 ? utils.now() : 0; + + // Discourage fee snipping a la bitcoind + // if (data.lock == null && this.chain) { + // this.lock = this.chain.height(); + // if ((Math.random() * 10 | 0) === 0) + // this.lock = Math.max(0, this.lock - (Math.random() * 100 | 0)); + // } } TX.prototype.clone = function clone() { @@ -92,6 +100,11 @@ TX.prototype.input = function input(i, index) { return this; }; +// tx._input(tx, index) +// tx._input(hash, index) +// tx._input(input) +// tx._input({ hash: hash, index: index }) +// tx._input({ tx: tx, index: index }) TX.prototype._input = function _input(obj, index) { var options, hash, input, ex, i; @@ -123,6 +136,14 @@ TX.prototype._input = function _input(obj, index) { seq: options.seq }); + // Locktime won't work without a seq < uint32max + // if (this.lock !== 0) + // input.seq = 0; + + // Replace by fee opt-in + // if (this.rbf) + // input.seq = 0xffffffff - 1; + // Try modifying existing input first i = this._inputIndex(input.out.hash, input.out.index); if (i !== -1) { @@ -252,14 +273,11 @@ TX.prototype.signature = function signature(index, key, type) { hash = this.signatureHash(index, s, type); // Sign the transaction with our one input - signature = bcoin.script.sign(hash, key); + signature = bcoin.script.sign(hash, key, type); // Something is broken if this doesn't work: assert(bcoin.script.checksig(hash, signature, key)); - // Add the sighash as a single byte to the signature - signature = signature.concat(type); - return signature; }; @@ -648,7 +666,12 @@ TX.prototype.signatureHash = function signatureHash(index, s, type) { assert(index >= 0 && index < copy.inputs.length) assert(Array.isArray(s)); - assert(utils.isFinite(type)); + + // Disable this for now. We allow null hash types + // because bitcoind allows empty signatures. On + // another note, we allow all weird sighash types + // if strictenc is not enabled. + // assert(utils.isFinite(type)); // Remove code separators. // s = script.subscript(s); diff --git a/test/wallet-test.js b/test/wallet-test.js index eecaf26c..bbfd3618 100644 --- a/test/wallet-test.js +++ b/test/wallet-test.js @@ -80,7 +80,8 @@ describe('Wallet', function() { n: 2 } }); - var k2 = w.getPublicKey().concat(1); + // var k2 = w.getPublicKey().concat(1); + var k2 = bcoin.ecdsa.genKeyPair().getPublic(true, 'array'); w.addKey(k2); assert.equal(w.getKeyAddress(), w.getAddress());