Steem bug: Private key format(wif) errors in steem-js library

Edit: it seems @drakos(right?) already opened the issue : https://github.com/steemit/steem-js/issues/383. Well now we know exactly where the bug came from with my post.

This bug is about implementation mistake of the WIF. No funds should have been lost because of it. What might have happened is a user saving an incorrect private key (as explain below) but the correct key is easily retrievable.

Project Information

• Repository: https://github.com/steemit/steem-js

Expected behavior

Wif private keys have features to stop users errors when dealing with private keys:

• base 58 to not write incorrect letters,
• a checksum to know the key is properly formatted and valid.

So changing one character in the private key has (very) low chances to give you another valid private key. Also you should get a different corresponding public address.

Actual behaviour

Below an example with a random key:

Private key and corresponding Public key
5J9GqSAGHtH8Yf8JWuWnWaGedB5WUtRCPFTThMPsLDuCQUVSD5a      STM5yabD4ScbVKc8EAjSaf9TMtasePQ6pwufMCQY8hnXvvWK8Gpue 

Starting Key iteration
5J9GqSAGHtH8Yf8JWuWnWaGedB5bUtRCPFTThMPsLDuCQUVSD5a      STM4uyNzGHyfE8Vcs4oNUnn3ehjRHzL8aUHnovghP8xGzixMzMLn3
5J9GqSAGHtH8Yf8JWuWnWaGedB5WUtECPFTThMPsLDuCQUVSD5a      STM7osWuir46FamHZBbhQmniD1TVSV3uWEFNUtf1rtfVrsLraumuK
5J9GqSAGHtH8Yf8JWuWnWaGedB5WUtRCPFTThMPsLDuCQUVSE5a      STM5yabD4ScbVKc8EAjSaf9TMtasePQ6pwufMCQY8hnXvvWK8Gpue
5J9GqSAGHtH8Yf8JWuWnWaGedB5WUtRCPFTThMPsLDuCQUVSF5a      STM5yabD4ScbVKc8EAjSaf9TMtasePQ6pwufMCQY8hnXvvWK8Gpue
5J9GqSAGHtH8Yf8JWuWnWaGedB5WUtRCPFTThMPsLDuCQUVSG5a      STM5yabD4ScbVKc8EAjSaf9TMtasePQ6pwufMCQY8hnXvvWK8Gpue
...

There are much more hits. You can see in the beginning a real collision: we find another valid WIF but the public key is, as expected different. The other cases should not happen.

You can test it with the following JS functions:

function trial(){
  const base58="123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz";
  const private_key="5J9GqSAGHtH8Yf8JWuWnWaGedB5WUtRCPFTThMPsLDuCQUVSD5a";
  console.log("Private key and corresponding Public key");
  console.log(private_key,"\t",steem.auth.wifToPublic(private_key),"\n");
  console.log("Starting Key iteration");
  var key;
  for (var i = 0; i < private_key.length; i++) {
    for (var j = 0; j < base58.length; j++) {
      key = setCharAt(private_key,i,base58[j]);
      if(steem.auth.isWif(key) && key !== private_key){
            console.log(key,"\t",steem.auth.wifToPublic(key));//," ",steem.auth.Signature);
      }
    }
  }
}

function setCharAt(str,index,chr) {
    if(index > str.length-1) return str;
    return str.substr(0,index) + chr + str.substr(index+1);
}

This code finds all the valid wif which differ from one base58 character from the starting key.

The problems

I found two problems while investigating this bug.
First of all, there were no need to have two function doing the same:

• https://github.com/steemit/steem-js/blob/master/src/auth/index.js#L63
• https://github.com/steemit/steem-js/blob/master/src/auth/ecc/src/key_private.js#L42
  Actually the second one does one more check so is better.

Second, comparing two buffers, when the value don't necessarily fit in ascii is a bad idea. Look at the the very detailed accepted explanation on Buffer comparison. In summary, the values unrecognised for ascii will become the Unicode Replacement Character thus become equal.

Small addition: from how base systems work, it makes sense most of the invalid keys will have the difference at the end.

Fix

Make a buffer comparison directly and clean up a bit the functions. Also add a recovery code (my bruteforce search) in case someone saved the wrong wif.

GitHub Account

https://github.com/cryptohazard/

Merci to the witness @evildido to make me work on that. It was a pretty intriguing case and I had to methodically look at each step of the algorithm until I got the intuition.