From 1e23085ad7e28172440c7335af1350977da58ca4 Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Fri, 21 Aug 2015 20:35:22 +1000 Subject: [PATCH 1/8] use bip66 module --- package.json | 1 + src/ecsignature.js | 52 +++++++--------------------------------------- 2 files changed, 8 insertions(+), 45 deletions(-) diff --git a/package.json b/package.json index d74bd03..538dd80 100644 --- a/package.json +++ b/package.json @@ -46,6 +46,7 @@ }, "dependencies": { "bigi": "^1.4.0", + "bip66": "^1.0.2", "bs58check": "^1.0.5", "create-hash": "^1.1.0", "create-hmac": "^1.1.3", diff --git a/src/ecsignature.js b/src/ecsignature.js index 940387b..4fd3b8f 100644 --- a/src/ecsignature.js +++ b/src/ecsignature.js @@ -1,3 +1,4 @@ +var bip66 = require('bip66') var typeforce = require('typeforce') var types = require('./types') @@ -29,36 +30,10 @@ ECSignature.parseCompact = function (buffer) { } } -// Strict DER - https://github.com/bitcoin/bips/blob/master/bip-0066.mediawiki -// NOTE: SIGHASH byte ignored ECSignature.fromDER = function (buffer) { - // Format: 0x30 [total-length] 0x02 [R-length] [R] 0x02 [S-length] [S] - if (buffer.length < 8) throw new Error('DER sequence too short') - if (buffer.length > 72) throw new Error('DER sequence too long') - if (buffer[0] !== 0x30) throw new Error('Not a DER sequence') - if (buffer[1] !== buffer.length - 2) throw new Error('Invalid sequence length') - if (buffer[2] !== 0x02) throw new Error('Expected a DER integer') - - var lenR = buffer[3] - if (lenR === 0) throw new Error('R length is zero') - if (5 + lenR >= buffer.length) throw new Error('Invalid DER encoding') - if (buffer[4 + lenR] !== 0x02) throw new Error('Expected a DER integer (2)') - - var lenS = buffer[5 + lenR] - if (lenS === 0) throw new Error('S length is zero') - if ((lenR + lenS + 6) !== buffer.length) throw new Error('Invalid DER encoding (2)') - - if (buffer[4] & 0x80) throw new Error('R value is negative') - if (lenR > 1 && (buffer[4] === 0x00) && !(buffer[5] & 0x80)) throw new Error('R value excessively padded') - - if (buffer[lenR + 6] & 0x80) throw new Error('S value is negative') - if (lenS > 1 && (buffer[lenR + 6] === 0x00) && !(buffer[lenR + 7] & 0x80)) throw new Error('S value excessively padded') - - // non-BIP66 - extract R, S values - var rB = buffer.slice(4, 4 + lenR) - var sB = buffer.slice(lenR + 6) - var r = BigInteger.fromDERInteger(rB) - var s = BigInteger.fromDERInteger(sB) + var decode = bip66.decode(buffer) + var r = BigInteger.fromDERInteger(decode.r) + var s = BigInteger.fromDERInteger(decode.s) return new ECSignature(r, s) } @@ -93,23 +68,10 @@ ECSignature.prototype.toCompact = function (i, compressed) { } ECSignature.prototype.toDER = function () { - var rBa = this.r.toDERInteger() - var sBa = this.s.toDERInteger() + var r = new Buffer(this.r.toDERInteger()) + var s = new Buffer(this.s.toDERInteger()) - var sequence = [] - - // INTEGER - sequence.push(0x02, rBa.length) - sequence = sequence.concat(rBa) - - // INTEGER - sequence.push(0x02, sBa.length) - sequence = sequence.concat(sBa) - - // SEQUENCE - sequence.unshift(0x30, sequence.length) - - return new Buffer(sequence) + return bip66.encode(r, s) } ECSignature.prototype.toScriptSignature = function (hashType) { From e1cb5e615273807853704c5e7b742788273930af Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Fri, 21 Aug 2015 20:39:24 +1000 Subject: [PATCH 2/8] amend ECSignature tests to reflect BIP66 module --- package.json | 2 +- test/bitcoin.core.js | 4 +--- test/fixtures/ecsignature.json | 36 +++++++++++++++++++--------------- 3 files changed, 22 insertions(+), 20 deletions(-) diff --git a/package.json b/package.json index 538dd80..b64098f 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ }, "dependencies": { "bigi": "^1.4.0", - "bip66": "^1.0.2", + "bip66": "^1.0.8", "bs58check": "^1.0.5", "create-hash": "^1.1.0", "create-hmac": "^1.1.3", diff --git a/test/bitcoin.core.js b/test/bitcoin.core.js index cdc3b8e..2757042 100644 --- a/test/bitcoin.core.js +++ b/test/bitcoin.core.js @@ -243,14 +243,12 @@ describe('Bitcoin-core', function () { if (i % 2 !== 0) return var description = sig_noncanonical[i - 1].slice(0, -1) - if (description === 'too long') return // we support non secp256k1 signatures - var buffer = new Buffer(hex, 'hex') it('throws on ' + description, function () { assert.throws(function () { bitcoin.ECSignature.parseScriptSignature(buffer) - }) + }, /Expected DER (integer|sequence)|(R|S) value (excessively padded|is negative)|(R|S|DER sequence) length is (zero|too short|too long|invalid)|Invalid hashType/) }) }) }) diff --git a/test/fixtures/ecsignature.json b/test/fixtures/ecsignature.json index 6f51590..363ae72 100644 --- a/test/fixtures/ecsignature.json +++ b/test/fixtures/ecsignature.json @@ -130,37 +130,33 @@ ], "DER": [ { - "exception": "DER sequence too short", + "exception": "DER sequence length is too short", "hex": "ffffffffffffff" }, { - "exception": "DER sequence too long", + "exception": "DER sequence length is too long", "hex": "ffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff" }, { - "exception": "Invalid sequence length", + "exception": "Expected DER sequence", + "hex": "00ffff0400ffffff020400ffffff" + }, + { + "exception": "DER sequence length is invalid", "hex": "30ff020400ffffff020400ffffff" }, { - "exception": "Invalid sequence length", + "exception": "DER sequence length is invalid", "hex": "300c030400ffffff030400ffffff0000" }, { - "exception": "Expected a DER integer", + "exception": "Expected DER integer", "hex": "300cff0400ffffff020400ffffff" }, { - "exception": "Expected a DER integer \\(2\\)", + "exception": "Expected DER integer \\(2\\)", "hex": "300c020200ffffff020400ffffff" }, - { - "exception": "Invalid DER encoding", - "hex": "300c0204ddffffff020200ffffff" - }, - { - "exception": "Invalid DER encoding \\(2\\)", - "hex": "300c020400ffffff02dd00ffffff" - }, { "exception": "R length is zero", "hex": "30080200020400ffffff" @@ -169,13 +165,21 @@ "exception": "S length is zero", "hex": "3008020400ffffff0200" }, + { + "exception": "R length is too long", + "hex": "300c02dd00ffffff020400ffffff" + }, + { + "exception": "S length is invalid", + "hex": "300c020400ffffff02dd00ffffff" + }, { "exception": "R value is negative", - "hex": "300c0204ffffffff020400ffffff" + "hex": "300c020480000000020400ffffff" }, { "exception": "S value is negative", - "hex": "300c020400ffffff0204ffffffff" + "hex": "300c020400ffffff020480000000" }, { "exception": "R value excessively padded", From abac254d0d2c6e57e937609c13ca1ac9cefeb51b Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 22 Aug 2015 11:54:00 +1000 Subject: [PATCH 3/8] script: use bip66 for signature checking --- package.json | 2 +- src/script.js | 14 ++------------ 2 files changed, 3 insertions(+), 13 deletions(-) diff --git a/package.json b/package.json index b64098f..a499bec 100644 --- a/package.json +++ b/package.json @@ -46,7 +46,7 @@ }, "dependencies": { "bigi": "^1.4.0", - "bip66": "^1.0.8", + "bip66": "^1.1.0", "bs58check": "^1.0.5", "create-hash": "^1.1.0", "create-hmac": "^1.1.3", diff --git a/src/script.js b/src/script.js index ffd4f14..783c5da 100644 --- a/src/script.js +++ b/src/script.js @@ -1,8 +1,8 @@ +var bip66 = require('bip66') var bufferutils = require('./bufferutils') var typeforce = require('typeforce') var types = require('./types') -var ECSignature = require('./ecsignature') var ecurve = require('ecurve') var curve = ecurve.getCurveByName('secp256k1') @@ -135,17 +135,7 @@ function isCanonicalPubKey (buffer) { function isCanonicalSignature (buffer) { if (!Buffer.isBuffer(buffer)) return false - try { - ECSignature.parseScriptSignature(buffer) - } catch (e) { - if (!(e.message.match(/Not a DER sequence|Invalid sequence length|Expected a DER integer|R length is zero|S length is zero|R value excessively padded|S value excessively padded|R value is negative|S value is negative|Invalid hashType/))) { - throw e - } - - return false - } - - return true + return bip66.check(buffer.slice(0, -1)) } function isPubKeyHashInput (script) { From 4be502a1bd88e2f67ab98e8d83d06212f94d20fc Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 22 Aug 2015 12:27:23 +1000 Subject: [PATCH 4/8] tests: add non-canonical pubkey fixtures --- test/fixtures/script.json | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/fixtures/script.json b/test/fixtures/script.json index 1a17622..f0136fd 100644 --- a/test/fixtures/script.json +++ b/test/fixtures/script.json @@ -178,10 +178,20 @@ ], "isPubKeyInput": [ { - "description": "non-canonical signature", + "description": "non-canonical signature (too long)", "scriptSig": "304402207515cf147d201f411092e6be5a64a6006f9308fad7b2a8fdaab22cd86ce764c202200974b8aca7bf51dbf54150d3884e1ae04f675637b926ec33bf7593ffffffffffffffff" } ], + "isPubKeyOutput": [ + { + "description": "non-canonical pubkey (too short)", + "scriptPubKey": "02359c6e3f04cefbf089cf1d6670dc47c3fb4df68e2bad1fa5a369f9ce OP_CHECKSIG" + }, + { + "description": "non-canonical pubkey (too long)", + "scriptPubKey": "02359c6e3f04cefbf089cf1d6670dc47c3fb4df68e2bad1fa5a369f9ce4b42bbd1ffffff OP_CHECKSIG" + } + ], "isMultisigOutput": [ { "description": "OP_CHECKMULTISIG not found", From 86b2cf75cecc47dba0ab116e59c5827c1118b262 Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 22 Aug 2015 12:29:58 +1000 Subject: [PATCH 5/8] tests: add failing test for undefined hashType --- test/fixtures/script.json | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/fixtures/script.json b/test/fixtures/script.json index f0136fd..59bb6e8 100644 --- a/test/fixtures/script.json +++ b/test/fixtures/script.json @@ -177,9 +177,17 @@ } ], "isPubKeyInput": [ + { + "description": "non-canonical signature (too short)", + "scriptSig": "304402207515cf147d201f411092e6be5a64a6006f9308fad7b2a8fdaab22cd86ce764c202200974b8aca7bf51dbf54150d3884e1ae04f675637b926ec33bf7593" + }, { "description": "non-canonical signature (too long)", - "scriptSig": "304402207515cf147d201f411092e6be5a64a6006f9308fad7b2a8fdaab22cd86ce764c202200974b8aca7bf51dbf54150d3884e1ae04f675637b926ec33bf7593ffffffffffffffff" + "scriptSig": "304402207515cf147d201f411092e6be5a64a6006f9308fad7b2a8fdaab22cd86ce764c202200974b8aca7bf51dbf54150d3884e1ae04f675637b926ec33bf75939446f6ca28ffffffff01" + }, + { + "description": "non-canonical signature (invalid hashType)", + "scriptSig": "304402207515cf147d201f411092e6be5a64a6006f9308fad7b2a8fdaab22cd86ce764c202200974b8aca7bf51dbf54150d3884e1ae04f675637b926ec33bf75939446f6ca28ff" } ], "isPubKeyOutput": [ From 0ff5bd569887f5ecfc7a8b184a3fa89e91b61ebc Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 22 Aug 2015 12:31:00 +1000 Subject: [PATCH 6/8] script: add isDefinedHashType to check hashType --- src/script.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/script.js b/src/script.js index 783c5da..f4f096c 100644 --- a/src/script.js +++ b/src/script.js @@ -134,10 +134,18 @@ function isCanonicalPubKey (buffer) { function isCanonicalSignature (buffer) { if (!Buffer.isBuffer(buffer)) return false + if (!isDefinedHashType(buffer[buffer.length - 1])) return false return bip66.check(buffer.slice(0, -1)) } +function isDefinedHashType (hashType) { + var hashTypeMod = hashType & ~0x80 + +// return hashTypeMod > SIGHASH_ALL && hashTypeMod < SIGHASH_SINGLE + return hashTypeMod > 0x00 && hashTypeMod < 0x04 +} + function isPubKeyHashInput (script) { var chunks = decompile(script) @@ -369,6 +377,7 @@ module.exports = { isCanonicalPubKey: isCanonicalPubKey, isCanonicalSignature: isCanonicalSignature, + isDefinedHashType: isDefinedHashType, isPubKeyHashInput: isPubKeyHashInput, isPubKeyHashOutput: isPubKeyHashOutput, isPubKeyInput: isPubKeyInput, From 3106fc13ca16d1b117952feb2b532bb41c633ecb Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 22 Aug 2015 12:31:32 +1000 Subject: [PATCH 7/8] script: refactor isCanonicalPubKey to bitcoin-core equivalent --- src/script.js | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/src/script.js b/src/script.js index f4f096c..d799b5b 100644 --- a/src/script.js +++ b/src/script.js @@ -118,18 +118,17 @@ function decompile (buffer) { function isCanonicalPubKey (buffer) { if (!Buffer.isBuffer(buffer)) return false + if (buffer.length < 33) return false - try { - ecurve.Point.decodeFrom(curve, buffer) - } catch (e) { - if (!(e.message.match(/Invalid sequence (length|tag)/))) { - throw e - } - - return false + switch (buffer[0]) { + case 0x02: + case 0x03: + return buffer.length === 33 + case 0x04: + return buffer.length === 65 } - return true + return false } function isCanonicalSignature (buffer) { From 92937a8ded95207a827eda63077e26e20042c6c7 Mon Sep 17 00:00:00 2001 From: Daniel Cousens Date: Sat, 22 Aug 2015 12:37:17 +1000 Subject: [PATCH 8/8] script: rm unused ecurve --- src/script.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/script.js b/src/script.js index d799b5b..b91997f 100644 --- a/src/script.js +++ b/src/script.js @@ -3,9 +3,6 @@ var bufferutils = require('./bufferutils') var typeforce = require('typeforce') var types = require('./types') -var ecurve = require('ecurve') -var curve = ecurve.getCurveByName('secp256k1') - var OPS = require('./opcodes') var REVERSE_OPS = [] for (var op in OPS) {