ethereumjs/keythereum

deriveKey silently fails for empty passwords

Closed this issue · 4 comments

maciejhirsz commented

deriveKey internally performs a if (password && salt) before proceeding. This works fine for most scenarios, but will fail when password is an empty string (which should still be a valid use case).

Additionally when it does fail, it does not throw an informative exception, it just returns undefined and tends to cause a confusing error somewhere down the line (calling slice of undefined and what not).

Suggested tweaks:

  • Change the password check to password != null or typeof password === 'string'.
  • When either password or salt are missing, throw an exception.
tinybike commented

Good catch, thanks!

o7-machinehum commented

Was this patched? I'm still having some issues with using an empty password

[machinehum@localhost geth]$ cat get_key.js 
var keythereum = require("keythereum");
const PASSWORD = '';

var keyObject = keythereum.importFromFile('aa2896c25489aa2629626d11481556d9a4bf59f4', 'node01');
var privateKey = keythereum.recover(PASSWORD, keyObject);
console.log(privateKey.toString('hex'));
[machinehum@localhost geth]$ node get_key.js 
/home/machinehum/geth/node_modules/scrypt-js/scrypt.js:274
            throw new Error('password must be an array or buffer');
            ^

Error: password must be an array or buffer
    at _scrypt (/home/machinehum/geth/node_modules/scrypt-js/scrypt.js:274:19)
    at Object.syncScrypt (/home/machinehum/geth/node_modules/scrypt-js/scrypt.js:463:35)
    at Object.deriveKeyUsingScrypt (/home/machinehum/geth/node_modules/keythereum/index.js:196:33)
    at Object.deriveKey (/home/machinehum/geth/node_modules/keythereum/index.js:225:19)
    at Object.recover (/home/machinehum/geth/node_modules/keythereum/index.js:430:36)
    at Object.<anonymous> (/home/machinehum/geth/get_key.js:5:29)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
o7-machinehum commented

Confirmed it works fine on a UTC w/ a key.

NicWickman commented

Passing an empty array var PASSWORD = []; works.