bitpay / bitcore-mnemonic

BIP39 Mnemonics implemented for Bitcore
http://bitcore.io
MIT License
155 stars 212 forks source link

Mnemonic._entropyChecksum sometimes returning wrong result in IE and Edge #30

Closed Flavien closed 8 years ago

Flavien commented 8 years ago

The result of toString on a number can be different across browsers for large numbers. In particular, it's possible for IE to use exponential notation, which breaks Mnemonic._entropyChecksum.

This can be reproduced using this seed: patrol wise idea oyster inquiry crash dignity chronic scatter time admit pet, which is valid on Chrome and Firefox, but invalid on IE11 and Edge.

I tried a quick fix (https://github.com/bitpay/bitcore-mnemonic/pull/29), but I'm not super familiar with BIP39, and calculating the checksum seems more complicated than what I initially thought.

Flavien commented 8 years ago

Actually, it's worse than that, on IE, the mnemonics generated by Bitcore have an invalid checksum. IE will find them valid, but other browsers won't (and vice versa).

matiu commented 8 years ago

Thanks a lot for the report. We will check it ASAP.

On Sat, Aug 22, 2015 at 7:17 AM, Flavien Charlon notifications@github.com wrote:

Actually, it's worse than that, on IE, the mnemonics generated by Bitcore have an invalid checksum. IE will find them valid, but other browsers won't (and vice versa).

— Reply to this email directly or view it on GitHub https://github.com/bitpay/bitcore-mnemonic/issues/30#issuecomment-133675128 .

BitPay.com

braydonf commented 8 years ago

There are tests against these vectors: https://github.com/trezor/python-mnemonic/blob/master/vectors.json

A possible patch:

diff --git a/lib/mnemonic.js b/lib/mnemonic.js                                                                                                                                                                         
index 70e3e7a..1c25faa 100644                                                                                                                                                                                          
--- a/lib/mnemonic.js                                                                                                                                                                                                  
+++ b/lib/mnemonic.js                                                                                                                                                                                                  
@@ -1,6 +1,7 @@
 'use strict';                                                                                                                                                                                                         

 var bitcore = require('bitcore');                                                                                                                                                                                     
+var BN = bitcore.crypto.BN;                                                                                                                                                                                           
 var unorm = require('unorm');                                                                                                                                                                                         
 var _ = bitcore.deps._;                                                                                                                                                                                               

@@ -274,11 +275,14 @@ Mnemonic._entropyChecksum = function(entropy) {
   var bits = entropy.length * 8;                                                                                                                                                                                      
   var cs = bits / 32;                                                                                                                                                                                                 

-  var hashbits = parseInt(hash.toString('hex'), 16).toString(2);                                                                                                                                                      
+  var hashbitsbn = new BN(hash.toString('hex'), 16);                                                                                                                                                                  
+  var hashbits = hashbitsbn.toString(2);                                                                                                                                                                              
+                                                                                                                                                                                                                      
   // zero pad the hash bits                                                                                                                                                                                           
-  hashbits = (new Array(256).join('0') + hashbits).slice(-256).slice(0, cs);                                                                                                                                          
+  var zerohashbits = (new Array(256).join('0') + hashbits).slice(-256);                                                                                                                                               
+  var checksum = zerohashbits.slice(0, cs);                                                                                                                                                                           

-  return hashbits;                                                                                                                                                                                                    
+  return checksum;                                                                                                                                                                                                    
 };                                                                                                                                                                                                                    

 module.exports = Mnemonic;

However, it could be useful to have a test that is breaking to reveal the problem.

matiu commented 8 years ago

fixed by https://github.com/bitpay/bitcore-mnemonic/pull/33

thanks for the report

Flavien commented 8 years ago

Awesome, thanks. Are you going to update the bower package?

braydonf commented 8 years ago

Should be in the v0.13.2 tag and release.

braydonf commented 8 years ago

Also if you can verify that this fixes the issue, that would be helpful.

Flavien commented 8 years ago

I've just checked, and it works now on Internet Explorer 11 and Edge. Thanks guys.