Proposing to have `flush()` method

[chiro-hiro]

_privKey and _pubKey are always accessible that's make me uncomfortable. I'm propose to have this one.

Wallet.prototype.flush = function () {
  assert(Buffer.isBuffer(this._privKey), 'Private key should be an instance of Buffer')
  this._privKey.fill(0x00);
  assert(Buffer.isBuffer(this._pubKey), 'Public key should be an instance of Buffer')
  this._pubKey.fill(0x00);
}

[holgerd77]

I think this might be a valuable addition to the API, yes.

[axic]

I'm not fully sure what problem this is solving, since the key is not cleared from memory.

[holgerd77]

@axic Is it really so easy to read things from memory with JavaScript?

[axic]

I am just confused what benefit this gives. If a user of this library can call .flush() cannot they just invalidate references to the wallet?

[holgerd77]

Hmm, that's the same with all libraries, theoretically you can use/access all (most) parts of the code but are encouraged to engage with the methods which are part of the public API, otherwise it's monkey-patching. 🐵 😄

[chiro-hiro]

I'm not fully sure what problem this is solving, since the key is not cleared from memory.

I've tried to do some experiments and observe result. We can't remove the value from the memory easily even using delete but we can overwrite it.

const readline = require('readline');
let mem = Buffer.from('112233445566778899aabbccddeeff', 'hex');

readline.emitKeypressEvents(process.stdin);

process.stdin.on('keypress', (str, key) => {

  if (str === '\u0003' || (key.name === 'c' && key.ctrl)) {
    process.exit();
  }

  let fillValue = ((Math.random() * 0xff) | 0) >>> 0;
  console.log('Fill buffer with:', fillValue.toString(16));
  mem.fill(fillValue)

});

[alcuadrado]

IMO having such a method would give a false sense of security to the users of this library. It's not easy to be sure that every copy of the key was erased, especially when the private key is used by external libraries.