Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Safe Buffer usage #790

Merged
merged 3 commits into from
May 23, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,7 @@
"merkle-lib": "^2.0.10",
"pushdata-bitcoin": "^1.0.1",
"randombytes": "^2.0.1",
"safe-buffer": "^5.0.1",
"typeforce": "^1.8.7",
"varuint-bitcoin": "^1.0.4",
"wif": "^2.0.1"
Expand Down
5 changes: 3 additions & 2 deletions src/address.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var Buffer = require('safe-buffer').Buffer
var bs58check = require('bs58check')
var bscript = require('./script')
var networks = require('./networks')
Expand All @@ -9,7 +10,7 @@ function fromBase58Check (address) {
if (payload.length < 21) throw new TypeError(address + ' is too short')
if (payload.length > 21) throw new TypeError(address + ' is too long')

var version = payload[0]
var version = payload.readUInt8(0)
var hash = payload.slice(1)

return { hash: hash, version: version }
Expand All @@ -18,7 +19,7 @@ function fromBase58Check (address) {
function toBase58Check (hash, version) {
typeforce(types.tuple(types.Hash160bit, types.UInt8), arguments)

var payload = new Buffer(21)
var payload = Buffer.allocUnsafe(21)
Copy link
Member

@fanatid fanatid Apr 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see two variants in this PR: Buffer.alloc(length) and Buffer.allocUnsafe(length). Can you choose one?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fanatid I've used Buffer.alloc all through the tests... and Buffer.allocUnsafe in parts of the code where were previously using new Buffer; so no change.

We should probably only need need Buffer.alloc in cases of user input no?

Copy link
Member

@fanatid fanatid Apr 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

allocUnsafe (from docs) uses internal memory pool, maybe we should use alloc everywhere? (at least in bitcoinjs-lib)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this code is tested well enough that we can use both?

payload.writeUInt8(version, 0)
hash.copy(payload, 1)

Expand Down
8 changes: 4 additions & 4 deletions src/block.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var Buffer = require('safe-buffer').Buffer
var bcrypto = require('./crypto')
var fastMerkleRoot = require('merkle-lib/fastRoot')
var typeforce = require('typeforce')
Expand Down Expand Up @@ -78,7 +79,7 @@ Block.prototype.byteLength = function (headersOnly) {
}

Block.fromHex = function (hex) {
return Block.fromBuffer(new Buffer(hex, 'hex'))
return Block.fromBuffer(Buffer.from(hex, 'hex'))
}

Block.prototype.getHash = function () {
Expand All @@ -98,7 +99,7 @@ Block.prototype.getUTCDate = function () {

// TODO: buffer, offset compatibility
Block.prototype.toBuffer = function (headersOnly) {
var buffer = new Buffer(this.byteLength(headersOnly))
var buffer = Buffer.allocUnsafe(this.byteLength(headersOnly))

var offset = 0
function writeSlice (slice) {
Expand Down Expand Up @@ -143,8 +144,7 @@ Block.prototype.toHex = function (headersOnly) {
Block.calculateTarget = function (bits) {
var exponent = ((bits & 0xff000000) >> 24) - 3
var mantissa = bits & 0x007fffff
var target = new Buffer(32)
target.fill(0)
var target = Buffer.alloc(32, 0)
target.writeUInt32BE(mantissa, 28 - exponent)
return target
}
Expand Down
13 changes: 5 additions & 8 deletions src/ecdsa.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,13 @@
var Buffer = require('safe-buffer').Buffer
var createHmac = require('create-hmac')
var typeforce = require('typeforce')
var types = require('./types')

var BigInteger = require('bigi')
var ECSignature = require('./ecsignature')

var ZERO = new Buffer([0])
var ONE = new Buffer([1])
var ZERO = Buffer.alloc(1, 0)
var ONE = Buffer.alloc(1, 1)

var ecurve = require('ecurve')
var secp256k1 = ecurve.getCurveByName('secp256k1')
Expand All @@ -19,15 +20,11 @@ function deterministicGenerateK (hash, x, checkSig) {
types.Function
), arguments)

var k = new Buffer(32)
var v = new Buffer(32)

// Step A, ignored as hash already provided
// Step B
v.fill(1)

// Step C
k.fill(0)
var k = Buffer.alloc(32, 0)
var v = Buffer.alloc(32, 1)

// Step D
k = createHmac('sha256', k)
Expand Down
9 changes: 4 additions & 5 deletions src/ecsignature.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,18 +58,17 @@ ECSignature.prototype.toCompact = function (i, compressed) {

i += 27

var buffer = new Buffer(65)
var buffer = Buffer.alloc(65)
buffer.writeUInt8(i, 0)

this.r.toBuffer(32).copy(buffer, 1)
this.s.toBuffer(32).copy(buffer, 33)

return buffer
}

ECSignature.prototype.toDER = function () {
var r = new Buffer(this.r.toDERInteger())
var s = new Buffer(this.s.toDERInteger())
var r = Buffer.from(this.r.toDERInteger())
var s = Buffer.from(this.s.toDERInteger())

return bip66.encode(r, s)
}
Expand All @@ -78,7 +77,7 @@ ECSignature.prototype.toScriptSignature = function (hashType) {
var hashTypeMod = hashType & ~0x80
if (hashTypeMod <= 0 || hashTypeMod >= 4) throw new Error('Invalid hashType ' + hashType)

var hashTypeBuffer = new Buffer(1)
var hashTypeBuffer = Buffer.alloc(1)
hashTypeBuffer.writeUInt8(hashType, 0)

return Buffer.concat([this.toDER(), hashTypeBuffer])
Expand Down
9 changes: 5 additions & 4 deletions src/hdnode.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var Buffer = require('safe-buffer').Buffer
var base58check = require('bs58check')
var bcrypto = require('./crypto')
var createHmac = require('create-hmac')
Expand Down Expand Up @@ -25,7 +26,7 @@ function HDNode (keyPair, chainCode) {

HDNode.HIGHEST_BIT = 0x80000000
HDNode.LENGTH = 78
HDNode.MASTER_SECRET = new Buffer('Bitcoin seed')
HDNode.MASTER_SECRET = Buffer.from('Bitcoin seed', 'utf8')

HDNode.fromSeedBuffer = function (seed, network) {
typeforce(types.tuple(types.Buffer, types.maybe(types.Network)), arguments)
Expand All @@ -48,7 +49,7 @@ HDNode.fromSeedBuffer = function (seed, network) {
}

HDNode.fromSeedHex = function (hex, network) {
return HDNode.fromSeedBuffer(new Buffer(hex, 'hex'), network)
return HDNode.fromSeedBuffer(Buffer.from(hex, 'hex'), network)
}

HDNode.fromBase58 = function (string, networks) {
Expand Down Expand Up @@ -168,7 +169,7 @@ HDNode.prototype.toBase58 = function (__isPrivate) {
// Version
var network = this.keyPair.network
var version = (!this.isNeutered()) ? network.bip32.private : network.bip32.public
var buffer = new Buffer(78)
var buffer = Buffer.allocUnsafe(78)

// 4 bytes: version bytes
buffer.writeUInt32BE(version, 0)
Expand Down Expand Up @@ -206,7 +207,7 @@ HDNode.prototype.derive = function (index) {
typeforce(types.UInt32, index)

var isHardened = index >= HDNode.HIGHEST_BIT
var data = new Buffer(37)
var data = Buffer.allocUnsafe(37)

// Hardened child
if (isHardened) {
Expand Down
7 changes: 4 additions & 3 deletions src/script.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var Buffer = require('safe-buffer').Buffer
var bip66 = require('bip66')
var pushdata = require('pushdata-bitcoin')
var typeforce = require('typeforce')
Expand Down Expand Up @@ -44,7 +45,7 @@ function compile (chunks) {
return accum + 1
}, 0.0)

var buffer = new Buffer(bufferSize)
var buffer = Buffer.allocUnsafe(bufferSize)
var offset = 0

chunks.forEach(function (chunk) {
Expand Down Expand Up @@ -142,7 +143,7 @@ function fromASM (asm) {
typeforce(types.Hex, chunkStr)

// data!
return new Buffer(chunkStr, 'hex')
return Buffer.from(chunkStr, 'hex')
}))
}

Expand All @@ -152,7 +153,7 @@ function toStack (chunks) {

return chunks.map(function (op) {
if (Buffer.isBuffer(op)) return op
if (op === OPS.OP_0) return new Buffer(0)
if (op === OPS.OP_0) return Buffer.allocUnsafe(0)

return scriptNumber.encode(op - OP_INT_BASE)
})
Expand Down
8 changes: 5 additions & 3 deletions src/script_number.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
var Buffer = require('safe-buffer').Buffer

function decode (buffer, maxLength, minimal) {
maxLength = maxLength || 4
minimal = minimal === undefined ? true : minimal
Expand All @@ -16,8 +18,8 @@ function decode (buffer, maxLength, minimal) {
var a = buffer.readUInt32LE(0)
var b = buffer.readUInt8(4)

if (b & 0x80) return -((b & ~0x80) * 0x100000000 + a)
return b * 0x100000000 + a
if (b & 0x80) return -(((b & ~0x80) * 0x100000000) + a)
return (b * 0x100000000) + a
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extra parentheses for readable?

Copy link
Contributor Author

@dcousens dcousens Apr 19, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

standard@latest warns on these, haven't bumped yet in repository though - indeed it is just readability

}

var result = 0
Expand All @@ -43,7 +45,7 @@ function scriptNumSize (i) {
function encode (number) {
var value = Math.abs(number)
var size = scriptNumSize(value)
var buffer = new Buffer(size)
var buffer = Buffer.allocUnsafe(size)
var negative = number < 0

for (var i = 0; i < size; ++i) {
Expand Down
5 changes: 4 additions & 1 deletion src/templates/multisig/input.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// OP_0 [signatures ...]

var Buffer = require('safe-buffer').Buffer
var bscript = require('../../script')
var typeforce = require('typeforce')
var OPS = require('bitcoin-ops')
Expand All @@ -21,6 +22,8 @@ function check (script, allowIncomplete) {
}
check.toJSON = function () { return 'multisig input' }

var EMPTY_BUFFER = Buffer.allocUnsafe(0)

function encodeStack (signatures, scriptPubKey) {
typeforce([partialSignature], signatures)

Expand All @@ -36,7 +39,7 @@ function encodeStack (signatures, scriptPubKey) {
}
}

return [].concat(new Buffer(0), signatures)
return [].concat(EMPTY_BUFFER, signatures)
}

function encode (signatures, scriptPubKey) {
Expand Down
1 change: 1 addition & 0 deletions src/templates/scripthash/input.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
// <scriptSig> {serialized scriptPubKey script}

var Buffer = require('safe-buffer').Buffer
var bscript = require('../../script')
var typeforce = require('typeforce')

Expand Down
9 changes: 7 additions & 2 deletions src/templates/witnesscommitment/output.js
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
// OP_RETURN {aa21a9ed} {commitment}

var Buffer = require('safe-buffer').Buffer
var bscript = require('../../script')
var types = require('../../types')
var typeforce = require('typeforce')
var OPS = require('bitcoin-ops')

var HEADER = new Buffer('aa21a9ed', 'hex')
var HEADER = Buffer.from('aa21a9ed', 'hex')

function check (script) {
var buffer = bscript.compile(script)
Expand All @@ -21,7 +22,11 @@ check.toJSON = function () { return 'Witness commitment output' }
function encode (commitment) {
typeforce(types.Hash256bit, commitment)

return bscript.compile([OPS.OP_RETURN, Buffer.concat([HEADER, commitment])])
var buffer = Buffer.allocUnsafe(36)
HEADER.copy(buffer, 0)
commitment.copy(buffer, 4)

return bscript.compile([OPS.OP_RETURN, buffer])
}

function decode (buffer) {
Expand Down
23 changes: 12 additions & 11 deletions src/transaction.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var Buffer = require('safe-buffer').Buffer
var bcrypto = require('./crypto')
var bscript = require('./script')
var bufferutils = require('./bufferutils')
Expand Down Expand Up @@ -35,11 +36,11 @@ Transaction.SIGHASH_ANYONECANPAY = 0x80
Transaction.ADVANCED_TRANSACTION_MARKER = 0x00
Transaction.ADVANCED_TRANSACTION_FLAG = 0x01

var EMPTY_SCRIPT = new Buffer(0)
var EMPTY_SCRIPT = Buffer.allocUnsafe(0)
var EMPTY_WITNESS = []
var ZERO = new Buffer('0000000000000000000000000000000000000000000000000000000000000000', 'hex')
var ONE = new Buffer('0000000000000000000000000000000000000000000000000000000000000001', 'hex')
var VALUE_UINT64_MAX = new Buffer('ffffffffffffffff', 'hex')
var ZERO = Buffer.from('0000000000000000000000000000000000000000000000000000000000000000', 'hex')
var ONE = Buffer.from('0000000000000000000000000000000000000000000000000000000000000001', 'hex')
var VALUE_UINT64_MAX = Buffer.from('ffffffffffffffff', 'hex')
var BLANK_OUTPUT = {
script: EMPTY_SCRIPT,
valueBuffer: VALUE_UINT64_MAX
Expand Down Expand Up @@ -298,7 +299,7 @@ Transaction.prototype.hashForSignature = function (inIndex, prevOutScript, hashT
}

// serialize and hash
var buffer = new Buffer(txTmp.__byteLength(false) + 4)
var buffer = Buffer.allocUnsafe(txTmp.__byteLength(false) + 4)
buffer.writeInt32LE(hashType, buffer.length - 4)
txTmp.__toBuffer(buffer, 0, false)

Expand All @@ -323,7 +324,7 @@ Transaction.prototype.hashForWitnessV0 = function (inIndex, prevOutScript, value
var hashSequence = ZERO

if (!(hashType & Transaction.SIGHASH_ANYONECANPAY)) {
tbuffer = new Buffer(36 * this.ins.length)
tbuffer = Buffer.allocUnsafe(36 * this.ins.length)
toffset = 0

this.ins.forEach(function (txIn) {
Expand All @@ -337,7 +338,7 @@ Transaction.prototype.hashForWitnessV0 = function (inIndex, prevOutScript, value
if (!(hashType & Transaction.SIGHASH_ANYONECANPAY) &&
(hashType & 0x1f) !== Transaction.SIGHASH_SINGLE &&
(hashType & 0x1f) !== Transaction.SIGHASH_NONE) {
tbuffer = new Buffer(4 * this.ins.length)
tbuffer = Buffer.allocUnsafe(4 * this.ins.length)
toffset = 0

this.ins.forEach(function (txIn) {
Expand All @@ -353,7 +354,7 @@ Transaction.prototype.hashForWitnessV0 = function (inIndex, prevOutScript, value
return sum + 8 + varSliceSize(output.script)
}, 0)

tbuffer = new Buffer(txOutsSize)
tbuffer = Buffer.allocUnsafe(txOutsSize)
toffset = 0

this.outs.forEach(function (out) {
Expand All @@ -365,15 +366,15 @@ Transaction.prototype.hashForWitnessV0 = function (inIndex, prevOutScript, value
} else if ((hashType & 0x1f) === Transaction.SIGHASH_SINGLE && inIndex < this.outs.length) {
var output = this.outs[inIndex]

tbuffer = new Buffer(8 + varSliceSize(output.script))
tbuffer = Buffer.allocUnsafe(8 + varSliceSize(output.script))
toffset = 0
writeUInt64(output.value)
writeVarSlice(output.script)

hashOutputs = bcrypto.hash256(tbuffer)
}

tbuffer = new Buffer(156 + varSliceSize(prevOutScript))
tbuffer = Buffer.allocUnsafe(156 + varSliceSize(prevOutScript))
toffset = 0

var input = this.ins[inIndex]
Expand Down Expand Up @@ -405,7 +406,7 @@ Transaction.prototype.toBuffer = function (buffer, initialOffset) {
}

Transaction.prototype.__toBuffer = function (buffer, initialOffset, __allowWitness) {
if (!buffer) buffer = new Buffer(this.__byteLength(__allowWitness))
if (!buffer) buffer = Buffer.allocUnsafe(this.__byteLength(__allowWitness))

var offset = initialOffset || 0
function writeSlice (slice) { offset += slice.copy(buffer, offset) }
Expand Down
7 changes: 4 additions & 3 deletions src/transaction_builder.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
var Buffer = require('safe-buffer').Buffer
var baddress = require('./address')
var bcrypto = require('./crypto')
var bscript = require('./script')
Expand Down Expand Up @@ -367,9 +368,9 @@ function prepareInput (input, kpPubKey, redeemScript, witnessValue, witnessScrip

function buildStack (type, signatures, pubKeys, allowIncomplete) {
if (type === scriptTypes.P2PKH) {
if (signatures.length === 1 && signatures[0] instanceof Buffer && pubKeys.length === 1) return bscript.pubKeyHash.input.encodeStack(signatures[0], pubKeys[0])
if (signatures.length === 1 && Buffer.isBuffer(signatures[0]) && pubKeys.length === 1) return bscript.pubKeyHash.input.encodeStack(signatures[0], pubKeys[0])
} else if (type === scriptTypes.P2PK) {
if (signatures.length === 1 && signatures[0] instanceof Buffer) return bscript.pubKey.input.encodeStack(signatures[0])
if (signatures.length === 1 && Buffer.isBuffer(signatures[0])) return bscript.pubKey.input.encodeStack(signatures[0])
} else if (type === scriptTypes.MULTISIG) {
if (signatures.length > 0) {
signatures = signatures.map(function (signature) {
Expand Down Expand Up @@ -515,7 +516,7 @@ TransactionBuilder.prototype.addInput = function (txHash, vout, sequence, prevOu
// is it a hex string?
if (typeof txHash === 'string') {
// transaction hashs's are displayed in reverse order, un-reverse it
txHash = new Buffer(txHash, 'hex').reverse()
txHash = Buffer.from(txHash, 'hex').reverse()

// is it a Transaction object?
} else if (txHash instanceof Transaction) {
Expand Down
Loading