diff options
| -rw-r--r-- | CHANGELOG.md | 1 | ||||
| -rw-r--r-- | app/scripts/account-import-strategies/index.js | 13 | ||||
| -rw-r--r-- | package-lock.json | 2 | ||||
| -rw-r--r-- | test/unit/app/account-import-strategies.spec.js | 62 | ||||
| -rw-r--r-- | test/unit/test-utils.js | 17 |
5 files changed, 77 insertions, 18 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index c8aac50ad..c4323dd86 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## Current Master +- Attempting to import an empty private key will now show a clear error. - Fix bug where metamask data would stop being written to disk after prolonged use - Fix bug where account reset did not work with custom RPC providers. - Fix bug where nonce mutex was never released diff --git a/app/scripts/account-import-strategies/index.js b/app/scripts/account-import-strategies/index.js index 96e2b5912..16ae224ea 100644 --- a/app/scripts/account-import-strategies/index.js +++ b/app/scripts/account-import-strategies/index.js @@ -16,7 +16,18 @@ const accountImporter = { strategies: { 'Private Key': (privateKey) => { - const stripped = ethUtil.stripHexPrefix(privateKey) + if (!privateKey) { + throw new Error('Cannot import an empty key.') + } + + const prefixed = ethUtil.addHexPrefix(privateKey) + const buffer = ethUtil.toBuffer(prefixed) + + if (!ethUtil.isValidPrivate(buffer)) { + throw new Error('Cannot import invalid private key.') + } + + const stripped = ethUtil.stripHexPrefix(prefixed) return stripped }, 'JSON File': (input, password) => { diff --git a/package-lock.json b/package-lock.json index 88047cc86..0e8ac8654 100644 --- a/package-lock.json +++ b/package-lock.json @@ -8262,6 +8262,7 @@ "resolved": "https://registry.npmjs.org/eth-sig-util/-/eth-sig-util-1.4.2.tgz", "integrity": "sha1-jZWCAsftuq6Dlwf7pvCf8ydgYhA=", "requires": { + "ethereumjs-abi": "git+https://github.com/ethereumjs/ethereumjs-abi.git#4ea2fdfed09e8f99117d9362d17c6b01b64a2bcf", "ethereumjs-util": "^5.1.1" }, "dependencies": { @@ -31331,6 +31332,7 @@ "resolved": "https://registry.npmjs.org/web3/-/web3-0.20.3.tgz", "integrity": "sha1-yqRDc9yIFayHZ73ba6cwc5ZMqos=", "requires": { + "bignumber.js": "git+https://github.com/frozeman/bignumber.js-nolookahead.git#57692b3ecfc98bbdd6b3a516cb2353652ea49934", "crypto-js": "^3.1.4", "utf8": "^2.1.1", "xhr2": "*", diff --git a/test/unit/app/account-import-strategies.spec.js b/test/unit/app/account-import-strategies.spec.js index 83cfaeb3e..216c2f698 100644 --- a/test/unit/app/account-import-strategies.spec.js +++ b/test/unit/app/account-import-strategies.spec.js @@ -1,31 +1,59 @@ const assert = require('assert') const path = require('path') -const accountImporter = require('../../../app/scripts/account-import-strategies/index') const ethUtil = require('ethereumjs-util') +const accountImporter = require('../../../app/scripts/account-import-strategies/index') +const { assertRejects } = require('../test-utils') describe('Account Import Strategies', function () { const privkey = '0x4cfd3e90fc78b0f86bf7524722150bb8da9c60cd532564d7ff43f5716514f553' const json = '{"version":3,"id":"dbb54385-0a99-437f-83c0-647de9f244c3","address":"a7f92ce3fba24196cf6f4bd2e1eb3db282ba998c","Crypto":{"ciphertext":"bde13d9ade5c82df80281ca363320ce254a8a3a06535bbf6ffdeaf0726b1312c","cipherparams":{"iv":"fbf93718a57f26051b292f072f2e5b41"},"cipher":"aes-128-ctr","kdf":"scrypt","kdfparams":{"dklen":32,"salt":"7ffe00488319dec48e4c49a120ca49c6afbde9272854c64d9541c83fc6acdffe","n":8192,"r":8,"p":1},"mac":"2adfd9c4bc1cdac4c85bddfb31d9e21a684e0e050247a70c5698facf6b7d4681"}}' - it('imports a private key and strips 0x prefix', async function () { - const importPrivKey = await accountImporter.importAccount('Private Key', [ privkey ]) - assert.equal(importPrivKey, ethUtil.stripHexPrefix(privkey)) - }) + describe('private key import', function () { + it('imports a private key and strips 0x prefix', async function () { + const importPrivKey = await accountImporter.importAccount('Private Key', [ privkey ]) + assert.equal(importPrivKey, ethUtil.stripHexPrefix(privkey)) + }) - it('fails when password is incorrect for keystore', async function () { - const wrongPassword = 'password2' + it('throws an error for empty string private key', async () => { + assertRejects(async function() { + await accountImporter.importAccount('Private Key', [ '' ]) + }, Error, 'no empty strings') + }) - try { - await accountImporter.importAccount('JSON File', [ json, wrongPassword]) - } catch (error) { - assert.equal(error.message, 'Key derivation failed - possibly wrong passphrase') - } - }) + it('throws an error for undefined string private key', async () => { + assertRejects(async function () { + await accountImporter.importAccount('Private Key', [ undefined ]) + }) + }) - it('imports json string and password to return a private key', async function () { - const fileContentsPassword = 'password1' - const importJson = await accountImporter.importAccount('JSON File', [ json, fileContentsPassword]) - assert.equal(importJson, '0x5733876abe94146069ce8bcbabbde2677f2e35fa33e875e92041ed2ac87e5bc7') + it('throws an error for undefined string private key', async () => { + assertRejects(async function () { + await accountImporter.importAccount('Private Key', []) + }) + }) + + it('throws an error for invalid private key', async () => { + assertRejects(async function () { + await accountImporter.importAccount('Private Key', [ 'popcorn' ]) + }) + }) }) + describe('JSON keystore import', function () { + it('fails when password is incorrect for keystore', async function () { + const wrongPassword = 'password2' + + try { + await accountImporter.importAccount('JSON File', [ json, wrongPassword]) + } catch (error) { + assert.equal(error.message, 'Key derivation failed - possibly wrong passphrase') + } + }) + + it('imports json string and password to return a private key', async function () { + const fileContentsPassword = 'password1' + const importJson = await accountImporter.importAccount('JSON File', [ json, fileContentsPassword]) + assert.equal(importJson, '0x5733876abe94146069ce8bcbabbde2677f2e35fa33e875e92041ed2ac87e5bc7') + }) + }) }) diff --git a/test/unit/test-utils.js b/test/unit/test-utils.js new file mode 100644 index 000000000..7d0ae4d91 --- /dev/null +++ b/test/unit/test-utils.js @@ -0,0 +1,17 @@ +const assert = require('assert') + +module.exports = { + assertRejects, +} + +// assert.rejects added in node v10 +async function assertRejects (asyncFn, regExp) { + let f = () => {} + try { + await asyncFn() + } catch (error) { + f = () => { throw error } + } finally { + assert.throws(f, regExp) + } +} |