steemit / steem-js

Steem.js the official JavaScript library for Steem blockchain
https://www.npmjs.com/package/steem
MIT License
472 stars 225 forks source link

BUG on `steem.auth.isWif()` #392

Open raphaelsoul opened 6 years ago

raphaelsoul commented 6 years ago

isWif() has a bug due to native javascript runtime.

Expected behavior

const fakePrivKey = '5Kcj7dncqhFKHFUH8MMrk5BvAvjnLTZRYZchk2Lhx4HdqVqq9J6';
const validPrivKey = '5Kcj7dncqhFKHFUH8MMrk5BvAvjnLTZRYZchk2Lhx4HdqVqq9J5';

The validPrivKey is generate by a scripts calls steem-js API. and edit last code to '6' we got an invalid privkey which should not pass checksum validation.

steem.auth.isWif('5Kcj7dncqhFKHFUH8MMrk5BvAvjnLTZRYZchk2Lhx4HdqVqq9J6'); // true
steem.auth.wifIsValid('5Kcj7dncqhFKHFUH8MMrk5BvAvjnLTZRYZchk2Lhx4HdqVqq9J6'); // false

Exept both to false.

Actual behavior

isWif return true to an modified privkey with broken checksum.

Environment information

can be reproduced both in browser and node.js

https://github.com/steemit/steem-js/blob/master/src/auth/ecc/src/key_private.js#L65 the bug occurs in this line

// <Buffer e3 2a 54 ff>
// <Buffer e3 2a 54 fe>
const sum = new Buffer([0xe3, 0x2a, 0x54, 0xff]);
const sum2 = new Buffer([0xe3, 0x2a, 0x54, 0xfe]);
console.log(sum, sum2, sum.toString() === sum.toString()); // expect false, but return true!
raphaelsoul commented 6 years ago

A workround example

const checkPrivKey = (privKey) => {
  return PrivateKey.fromWif(privKey).toString() === privKey;
}
raphaelsoul commented 6 years ago

have simply fixed here. Could I send a PR? https://github.com/steemit/steem-js/commit/acff06b039d5883e4e2395d10014a67ba8130d77?diff=split