Merge pull request #245 from ethereumjs/enforce-buffer-accounts

Enforce buffer inputs for account methods

src/account.ts

@@ -2,9 +2,9 @@ const ethjsUtil = require('ethjs-util')
 import * as assert from 'assert'
 import * as secp256k1 from 'secp256k1'
 import * as BN from 'bn.js'
-import { toBuffer, zeros, bufferToHex } from './bytes'
+import { zeros, bufferToHex } from './bytes'
 import { keccak, keccak256, rlphash } from './hash'
-import { assertIsHexString } from './helpers'
+import { assertIsHexString, assertIsBuffer } from './helpers'
 
 /**
  * Returns a zero address.
@@ -80,7 +80,8 @@ export const isValidChecksumAddress = function(
  * @param nonce The nonce of the from account
  */
 export const generateAddress = function(from: Buffer, nonce: Buffer): Buffer {
-  from = toBuffer(from)
+  assertIsBuffer(from)
+  assertIsBuffer(nonce)
   const nonceBN = new BN(nonce)
 
   if (nonceBN.isZero()) {
@@ -99,20 +100,16 @@ export const generateAddress = function(from: Buffer, nonce: Buffer): Buffer {
  * @param salt A salt
  * @param initCode The init code of the contract being created
  */
-export const generateAddress2 = function(
-  from: Buffer | string,
-  salt: Buffer | string,
-  initCode: Buffer | string,
-): Buffer {
-  const fromBuf = toBuffer(from)
-  const saltBuf = toBuffer(salt)
-  const initCodeBuf = toBuffer(initCode)
-
-  assert(fromBuf.length === 20)
-  assert(saltBuf.length === 32)
+export const generateAddress2 = function(from: Buffer, salt: Buffer, initCode: Buffer): Buffer {
+  assertIsBuffer(from)
+  assertIsBuffer(salt)
+  assertIsBuffer(initCode)
+
+  assert(from.length === 20)
+  assert(salt.length === 32)
 
   const address = keccak256(
-    Buffer.concat([Buffer.from('ff', 'hex'), fromBuf, saltBuf, keccak256(initCodeBuf)]),
+    Buffer.concat([Buffer.from('ff', 'hex'), from, salt, keccak256(initCode)]),
   )
 
   return address.slice(-20)
@@ -151,7 +148,7 @@ export const isValidPublic = function(publicKey: Buffer, sanitize: boolean = fal
  * @param sanitize Accept public keys in other formats
  */
 export const pubToAddress = function(pubKey: Buffer, sanitize: boolean = false): Buffer {
-  pubKey = toBuffer(pubKey)
+  assertIsBuffer(pubKey)
   if (sanitize && pubKey.length !== 64) {
     pubKey = secp256k1.publicKeyConvert(pubKey, false).slice(1)
   }
@@ -174,7 +171,7 @@ export const privateToAddress = function(privateKey: Buffer): Buffer {
  * @param privateKey A private key must be 256 bits wide
  */
 export const privateToPublic = function(privateKey: Buffer): Buffer {
-  privateKey = toBuffer(privateKey)
+  assertIsBuffer(privateKey)
   // skip the type flag and use the X, Y points
   return secp256k1.publicKeyCreate(privateKey, false).slice(1)
 }
@@ -183,7 +180,7 @@ export const privateToPublic = function(privateKey: Buffer): Buffer {
  * Converts a public key to the Ethereum format.
  */
 export const importPublic = function(publicKey: Buffer): Buffer {
-  publicKey = toBuffer(publicKey)
+  assertIsBuffer(publicKey)
   if (publicKey.length !== 64) {
     publicKey = secp256k1.publicKeyConvert(publicKey, false).slice(1)
   }

src/helpers.ts

@@ -10,3 +10,14 @@ export const assertIsHexString = function(input: string): void {
     throw new Error(msg)
   }
 }
+
+/**
+ * Throws if input is not a buffer
+ * @param {Buffer} input value to check
+ */
+export const assertIsBuffer = function(input: Buffer): void {
+  const msg = `This method only supports Buffer but input was: ${input}`
+  if (!Buffer.isBuffer(input)) {
+    throw new Error(msg)
+  }
+}

test/account.spec.ts

@@ -121,6 +121,11 @@ describe('importPublic', function() {
     const tmp = '033a443d8381a6798a70c6ff9304bdc8cb0163c23211d11628fae52ef9e0dca11a'
     assert.equal(importPublic(Buffer.from(tmp, 'hex')).toString('hex'), pubKey)
   })
+  it('should throw if input is not Buffer', function() {
+    assert.throws(function() {
+      importPublic((<unknown>pubKey) as Buffer)
+    })
+  })
 })
 
 describe('publicToAddress', function() {
@@ -160,15 +165,12 @@ describe('publicToAddress', function() {
       publicToAddress(pubKey)
     })
   })
-})
-
-describe('publicToAddress 0x', function() {
-  it('should produce an address given a public key', function() {
+  it('should throw if input is not a buffer', function() {
     const pubKey: any =
       '0x3a443d8381a6798a70c6ff9304bdc8cb0163c23211d11628fae52ef9e0dca11a001cf066d56a8156fc201cd5df8a36ef694eecd258903fca7086c1fae7441e1d'
-    const address = '2f015c60e0be116b1f0cd534704db9c92118fb6a'
-    const r = publicToAddress(pubKey)
-    assert.equal(r.toString('hex'), address)
+    assert.throws(function() {
+      publicToAddress(pubKey)
+    })
   })
 })
 
@@ -289,6 +291,16 @@ describe('privateToPublic', function() {
       privateToPublic(privateKey2)
     })
   })
+
+  it('should throw if private key is not Buffer', function() {
+    const privateKey = '0xea54bdc52d163f88c93ab0615782cf718a2efb9e51a7989aab1b08067e9c1c5f'
+    try {
+      privateToPublic((<unknown>privateKey) as Buffer)
+    } catch (err) {
+      assert(err.message.includes('This method only supports Buffer'))
+      assert(err.message.includes(privateKey))
+    }
+  })
 })
 
 describe('privateToAddress', function() {
@@ -364,16 +376,71 @@ describe('generateAddress with nonce 0 (special case)', function() {
   })
 })
 
+describe('generateAddress with non-buffer inputs', function() {
+  it('should throw if address is not Buffer', function() {
+    assert.throws(function() {
+      generateAddress(
+        (<unknown>'0x990ccf8a0de58091c028d6ff76bb235ee67c1c39') as Buffer,
+        toBuffer(0),
+      )
+    })
+  })
+  it('should throw if nonce is not Buffer', function() {
+    assert.throws(function() {
+      generateAddress(
+        toBuffer('0x990ccf8a0de58091c028d6ff76bb235ee67c1c39'),
+        (<unknown>0) as Buffer,
+      )
+    })
+  })
+})
+
 describe('generateAddress2: EIP-1014 testdata examples', function() {
   for (let i = 0; i <= 6; i++) {
     let e = eip1014Testdata[i]
     it(`${e['comment']}: should generate the addresses provided`, function() {
-      let result = generateAddress2(e['address'], e['salt'], e['initCode'])
+      let result = generateAddress2(
+        toBuffer(e['address']),
+        toBuffer(e['salt']),
+        toBuffer(e['initCode']),
+      )
       assert.equal('0x' + result.toString('hex'), e['result'])
     })
   }
 })
 
+describe('generateAddress2: non-buffer inputs', function() {
+  const e = eip1014Testdata[0]
+
+  it('should throw if address is not Buffer', function() {
+    assert.throws(function() {
+      generateAddress2(
+        (<unknown>e['address']) as Buffer,
+        toBuffer(e['salt']),
+        toBuffer(e['initCode']),
+      )
+    })
+  })
+  it('should throw if salt is not Buffer', function() {
+    assert.throws(function() {
+      generateAddress2(
+        toBuffer(e['address']),
+        (<unknown>e['salt']) as Buffer,
+        toBuffer(e['initCode']),
+      )
+    })
+  })
+  it('should throw if initCode is not Buffer', function() {
+    assert.throws(function() {
+      generateAddress2(
+        toBuffer(e['address']),
+        toBuffer(e['salt']),
+        (<unknown>e['initCode']) as Buffer,
+      )
+    })
+  })
+})
+
 const eip55ChecksumAddresses = [
   // All caps
   '0x52908400098527886E0F7030069857D2E4169EE7',