[SERIOUS] Incorrect address is generated when pubkey starts with zero byte

[gmart7t2]

There's a bug that causes the wrong address to be generated if the wallet's public key starts with a zero byte. This is the case for 1 in 256 wallets.

This can lead the user to send funds to an address for which he doesn't have the private key, causing loss of funds. I was lucky and used a different program to generate the address from the seed phrase so my funds were recoverable.

Here's a demonstration. I cloned the repository and built it:

$ git clone https://github.com/CATProtocol/cat-token-box.git
Cloning into 'cat-token-box'...
Resolving deltas: 100% (365/365), done.
$ cd cat-token-box/
$ yarn install
Done in 16.20s.
$ yarn build
Done in 92.01s.

I copied in a wallet that has a public key that starts with a zero byte, displayed its address and balance. The address is wrong, and the balance appears to be empty:

$ cd packages/cli/
$ cp /tmp/wallet.json .
$ yarn cli wallet address
yarn run v1.22.22
$ node dist/main wallet address
Your address is bc1plqchqpu9js8mfx9xt5ssm8qc0yj25am55t3pw7qwqxf6t7wnfpcs5tlq9q
Done in 2.02s.
$ yarn cli wallet balances
yarn run v1.22.22
$ node dist/main wallet balances
No tokens found!
Done in 1.75s.

I replaced one of the module files with a modified version, and now a different address is generated, and the balances are displayed correctly:

$ mv ../../node_modules/bn.js/lib/bn.js{,-orig}
$ cp /tmp/bn.js ../../node_modules/bn.js/lib/
$ yarn cli wallet address
yarn run v1.22.22
$ node dist/main wallet address
Your address is bc1pw7etc86w7962avsksdr67095qx4xh003yefa5h4k7mprce4ftv6qlhsa3d
Done in 1.69s.
$ yarn cli wallet balances
yarn run v1.22.22
$ node dist/main wallet balances
┌──────────────────────────────────────────────────────────────────────┬─────────┬─────────────┐
│ tokenId                                                              │ symbol  │ balance     │
┼──────────────────────────────────────────────────────────────────────┼─────────┼─────────────┤
│ '77ab996ff19fa4453e6ee7b7af55f1fb136030653b6d39928e9033e650160588_0' │ 'Korat' │ '234000.00' │
│ '9c403e7064c2ff050a0a4bcc542e61b7fa505686c7c2dd31858f6812ee24fbc6_0' │ '🐉'    │ '3.00'      │
┴──────────────────────────────────────────────────────────────────────┴─────────┴─────────────┘

Done in 1.79s.

Here's the change in the modified module file:

$ diff -u ../../node_modules/bn.js/lib/bn.js{-orig,}
--- ../../node_modules/bn.js/lib/bn.js-orig 2024-10-27 16:13:58.332281760 -0300
+++ ../../node_modules/bn.js/lib/bn.js  2024-10-27 16:19:13.420015704 -0300
@@ -527,6 +527,16 @@

   BN.prototype.toBuffer = function toBuffer (endian, length) {
     assert(typeof Buffer !== 'undefined');
+    if (typeof endian === 'object') {
+        if (JSON.stringify(endian) == '{"size":32}') {
+            endian = 'be';
+            length = 32;
+        } else {
+            console.log("node_modules/bn.js/lib/bn.js toBuffer() expects string as 1st parameter, not", endian);
+            console.trace();
+            throw new Error("bad toBuffer() call");
+        }
+    }
     return this.toArrayLike(Buffer, endian, length);
   };

$ 

BN.toBuffer() is being called with {size:32} as its only parameter, but the length is meant to be in the 2nd parameter.

There is a different BN.toBuffer() in node_modules/bitcore-lib-inquisition/lib/crypto/bn.js which is likely what is meant to be being called.

[gmart7t2]

I didn't provide a seed phrase that triggers the bug. So here's one:

$ cat wallet.json 
{
  "accountPath": "m/86'/0'/0'/0/0",
  "name": "cat",
  "mnemonic": "scale scale scale scale scale scale scale scale scale scale scale scale"
}

Bad address:

$ yarn cli wallet address
yarn run v1.22.22
$ node dist/main wallet address
Your address is bc1pg50hvw3nu9vhkavugm7sjktv6zn7l09zheq3dujkh7nvxc845y7q9nedp2
Done in 1.76s.

Good address:

$ yarn cli wallet address
yarn run v1.22.22
$ node dist/main wallet address
Your address is bc1pttc5kl78vuhhknkp99ersd8r275nwsjf977qdf58s5e28494e4jq596mnp
Done in 1.74s.
$