Merge pull request #95 from michaelsbradleyjr/fix/tov3-salt-buffer

Normalize salt, iv, uuid params of .toV3()  before encrypting

.npmrc

@@ -0,0 +1 @@
+package-lock = false

.travis.yml

@@ -1,6 +1,5 @@
 language: node_js
 node_js:
-  - "6"
   - "8"
   - "10"
 addons:

package.json

@@ -60,8 +60,11 @@
     "@types/bn.js": "^4.11.5",
     "@types/mocha": "^5.2.7",
     "@types/node": "^12.0.10",
+    "@types/lodash.zip": "^4.2.6",
     "coveralls": "^3.0.0",
+    "ethers": "^4.0.33",
     "husky": "^2.1.0",
+    "lodash.zip": "^4.2.0",
     "mocha": "^5.2.0",
     "nyc": "^14.1.1",
     "prettier": "^1.15.3",

src/index.ts

@@ -9,6 +9,19 @@ const uuidv4 = require('uuid/v4')
 // parameters for the toV3() method
 
 interface V3Params {
+  kdf: string
+  cipher: string
+  salt: string | Buffer
+  iv: string | Buffer
+  uuid: string | Buffer
+  dklen: number
+  c: number
+  n: number
+  r: number
+  p: number
+}
+
+interface V3ParamsStrict {
   kdf: string
   cipher: string
   salt: Buffer
@@ -21,8 +34,43 @@ interface V3Params {
   p: number
 }
 
-function mergeToV3ParamsWithDefaults(params?: Partial<V3Params>): V3Params {
-  const v3Defaults: V3Params = {
+function validateHexString(paramName: string, str: string, length?: number) {
+  if (str.toLowerCase().startsWith('0x')) {
+    str = str.slice(2)
+  }
+  if (!str && !length) {
+    return str
+  }
+  if ((length as number) % 2) {
+    throw new Error(`Invalid length argument, must be an even number`)
+  }
+  if (typeof length === 'number' && str.length !== length) {
+    throw new Error(`Invalid ${paramName}, string must be ${length} hex characters`)
+  }
+  if (!/^([0-9a-f]{2})+$/i.test(str)) {
+    const howMany = typeof length === 'number' ? length : 'empty or a non-zero even number of'
+    throw new Error(`Invalid ${paramName}, string must be ${howMany} hex characters`)
+  }
+  return str
+}
+
+function validateBuffer(paramName: string, buff: Buffer, length?: number) {
+  if (!Buffer.isBuffer(buff)) {
+    const howManyHex =
+      typeof length === 'number' ? `${length * 2}` : 'empty or a non-zero even number of'
+    const howManyBytes = typeof length === 'number' ? ` (${length} bytes)` : ''
+    throw new Error(
+      `Invalid ${paramName}, must be a string (${howManyHex} hex characters) or buffer${howManyBytes}`,
+    )
+  }
+  if (typeof length === 'number' && buff.length !== length) {
+    throw new Error(`Invalid ${paramName}, buffer must be ${length} bytes`)
+  }
+  return buff
+}
+
+function mergeToV3ParamsWithDefaults(params?: Partial<V3Params>): V3ParamsStrict {
+  const v3Defaults: V3ParamsStrict = {
     cipher: 'aes-128-ctr',
     kdf: 'scrypt',
     salt: randomBytes(32),
@@ -38,17 +86,30 @@ function mergeToV3ParamsWithDefaults(params?: Partial<V3Params>): V3Params {
   if (!params) {
     return v3Defaults
   }
+
+  if (typeof params.salt === 'string') {
+    params.salt = Buffer.from(validateHexString('salt', params.salt), 'hex')
+  }
+  if (typeof params.iv === 'string') {
+    params.iv = Buffer.from(validateHexString('iv', params.iv, 32), 'hex')
+  }
+  if (typeof params.uuid === 'string') {
+    params.uuid = Buffer.from(validateHexString('uuid', params.uuid, 32), 'hex')
+  }
+
+  if (params.salt) {
+    validateBuffer('salt', params.salt)
+  }
+  if (params.iv) {
+    validateBuffer('iv', params.iv, 16)
+  }
+  if (params.uuid) {
+    validateBuffer('uuid', params.uuid, 16)
+  }
+
   return {
-    cipher: params.cipher || 'aes-128-ctr',
-    kdf: params.kdf || 'scrypt',
-    salt: params.salt || randomBytes(32),
-    iv: params.iv || randomBytes(16),
-    uuid: params.uuid || randomBytes(16),
-    dklen: params.dklen || 32,
-    c: params.c || 262144,
-    n: params.n || 262144,
-    r: params.r || 8,
-    p: params.p || 1,
+    ...v3Defaults,
+    ...(params as V3ParamsStrict),
   }
 }
 
@@ -60,6 +121,14 @@ const enum KDFFunctions {
 }
 
 interface ScryptKDFParams {
+  dklen: number
+  n: number
+  p: number
+  r: number
+  salt: Buffer
+}
+
+interface ScryptKDFParamsOut {
   dklen: number
   n: number
   p: number
@@ -68,27 +137,35 @@ interface ScryptKDFParams {
 }
 
 interface PBKDFParams {
+  c: number
+  dklen: number
+  prf: string
+  salt: Buffer
+}
+
+interface PBKDFParamsOut {
   c: number
   dklen: number
   prf: string
   salt: string
 }
 
 type KDFParams = ScryptKDFParams | PBKDFParams
+type KDFParamsOut = ScryptKDFParamsOut | PBKDFParamsOut
 
-function kdfParamsForPBKDF(opts: V3Params): PBKDFParams {
+function kdfParamsForPBKDF(opts: V3ParamsStrict): PBKDFParams {
   return {
     dklen: opts.dklen,
-    salt: opts.salt.toString('hex'),
+    salt: opts.salt,
     c: opts.c,
     prf: 'hmac-sha256',
   }
 }
 
-function kdfParamsForScrypt(opts: V3Params): ScryptKDFParams {
+function kdfParamsForScrypt(opts: V3ParamsStrict): ScryptKDFParams {
   return {
     dklen: opts.dklen,
-    salt: opts.salt.toString('hex'),
+    salt: opts.salt,
     n: opts.n,
     r: opts.r,
     p: opts.p,
@@ -130,7 +207,7 @@ interface V3Keystore {
     }
     ciphertext: string
     kdf: string
-    kdfparams: KDFParams
+    kdfparams: KDFParamsOut
     mac: string
   }
   id: string
@@ -361,6 +438,7 @@ export default class Wallet {
 
   // public instance methods
 
+  // tslint:disable-next-line
   public getPrivateKey(): Buffer {
     return this.privKey
   }
@@ -369,6 +447,7 @@ export default class Wallet {
     return ethUtil.bufferToHex(this.privKey)
   }
 
+  // tslint:disable-next-line
   public getPublicKey(): Buffer {
     return this.pubKey
   }
@@ -394,16 +473,16 @@ export default class Wallet {
       throw new Error('This is a public key only wallet')
     }
 
-    const v3Params: V3Params = mergeToV3ParamsWithDefaults(opts)
+    const v3Params: V3ParamsStrict = mergeToV3ParamsWithDefaults(opts)
 
-    let kdfParams: PBKDFParams | ScryptKDFParams
+    let kdfParams: KDFParams
     let derivedKey: Buffer
     switch (v3Params.kdf) {
       case KDFFunctions.PBKDF:
         kdfParams = kdfParamsForPBKDF(v3Params)
         derivedKey = crypto.pbkdf2Sync(
           Buffer.from(password),
-          v3Params.salt,
+          kdfParams.salt,
           kdfParams.c,
           kdfParams.dklen,
           'sha256',
@@ -414,7 +493,7 @@ export default class Wallet {
         // FIXME: support progress reporting callback
         derivedKey = scryptsy(
           Buffer.from(password),
-          v3Params.salt,
+          kdfParams.salt,
           kdfParams.n,
           kdfParams.r,
           kdfParams.p,
@@ -449,7 +528,10 @@ export default class Wallet {
         cipherparams: { iv: v3Params.iv.toString('hex') },
         cipher: v3Params.cipher,
         kdf: v3Params.kdf,
-        kdfparams: kdfParams,
+        kdfparams: {
+          ...kdfParams,
+          salt: kdfParams.salt.toString('hex'),
+        },
         mac: mac.toString('hex'),
       },
     }