Fix/reject unsafe segwit ver addresses

__tests__/integration/legacy-wallet.test.js

@@ -32,6 +32,25 @@ describe('LegacyWallet', function () {
     assert.ok(!w.isAddressValid('12345'));
     assert.ok(w.isAddressValid('bc1quuafy8htjjj263cvpj7md84magzmc8svmh8lrm'));
     assert.ok(w.isAddressValid('BC1QH6TF004TY7Z7UN2V5NTU4MKF630545GVHS45U7'));
+
+    // taproot:
+    assert.ok(!w.isAddressValid('bc1pw5dgrnzv')); // v1, data length != 32
+    assert.ok(!w.isAddressValid('bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7v8n0nx0muaewav253zgeav')); // v1, data length != 32
+    assert.ok(!w.isAddressValid('bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqh2y7hd')); // P2TR example with errors (using Bech32 instead of Bech32m)
+    assert.ok(!w.isAddressValid('bc1p38j9r5y49hruaue7wxjce0updqjuyyx0kh56v8s25huc6995vvpql3jow4')); // invalid char
+    assert.ok(!w.isAddressValid('BC130XLXVLHEMJA6C4DQV22UAPCTQUPFHLXM9H8Z3K2E72Q4K9HCZ7VQ7ZWS8R')); // invalid char
+    assert.ok(!w.isAddressValid('bc1pllllllllllllllllllllllllllllllllllllllllllllallllscqlhrddu')); // X is modulo P + 1 (invalid X, but 1 is valid, testing if wrapped modulo (P+1 mod P === 1) will pass)
+    assert.ok(!w.isAddressValid('bc1pllllllllllllllllllllllllllllllllllllllllllllallllshqcgyklh')); // X is modulo P - 1 (invalid X)
+    assert.ok(!w.isAddressValid('bc1pqtllllllllllllllllllllllllllllllllllllllllllllhlll7zcsqylfl')); // data length is 33 (valid point in compressed DER format (33 bytes))
+    assert.ok(!w.isAddressValid('bc1plllllllllllllllllllllllllllllllllllllllllll0lllu9cegrnmx')); // data is length 31 (valid X value with leading 0x00 trimmed)
+
+    assert.ok(w.isAddressValid('bc1pw38ttcljvgv9x64xpsq99dl9auy8vv50n25xcstuj2cagzcpx3us2m25kg'));
+    assert.ok(w.isAddressValid('bc1pqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqszqgpqyqsyjer9e'));
+    assert.ok(w.isAddressValid('bc1pmfr3p9j00pfxjh0zmgp99y8zftmd3s5pmedqhyptwy6lm87hf5sspknck9'));
+    assert.ok(w.isAddressValid('bc1p0xlxvlhemja6c4dqv22uapctqupfhlxm9h8z3k2e72q4k9hcz7vqzk5jj0'));
+
+    assert.ok(!w.isAddressValid('BC1SW50QGDZ25J')); // v16, valid but unsafe
+    assert.ok(!w.isAddressValid('bc1zw508d6qejxtdg4y5r3zarvaryvaxxpcs')); // v2, valid but unsafe
   });
 
   it('can fetch balance', async () => {

package.json

@@ -24,12 +24,13 @@
     "bignumber.js": "^9.0.1",
     "bip32": "^2.0.6",
     "bip39": "3.0.4",
-    "bitcoinjs-lib": "^5.2.0",
+    "bitcoinjs-lib": "^6.0.1",
     "bs58check": "^2.1.2",
     "buffer-reverse": "^1.0.1",
     "chacha": "^2.1.0",
     "coinselect": "^3.1.12",
     "create-hash": "^1.2.0",
+    "ecpair": "^2.0.1",
     "electrum-mnemonic": "^2.0.0",
     "frisbee": "^3.1.2",
     "util": "^0.12.3"

src/wallet/abstract-hd-electrum-wallet.js

@@ -7,6 +7,9 @@ import * as bitcoin from 'bitcoinjs-lib';
 import * as BlueElectrum from '../BlueElectrum';
 import * as HDNode from 'bip32';
 import reverse from 'buffer-reverse';
+import { ECPairFactory } from 'ecpair';
+const ecc = require('tiny-secp256k1');
+const ECPair = ECPairFactory(ecc);
 
 /**
  * Electrum - means that it utilizes Electrum protocol for blockchain data
@@ -853,7 +856,7 @@ export class AbstractHDElectrumWallet extends AbstractHDWallet {
       let keyPair;
       if (!skipSigning) {
         // skiping signing related stuff
-        keyPair = bitcoin.ECPair.fromWIF(this._getWifForAddress(input.address));
+        keyPair = ECPair.fromWIF(this._getWifForAddress(input.address));
         keypairs[c] = keyPair;
       }
       values[c] = input.value;

src/wallet/hd-segwit-p2sh-wallet.js

@@ -3,6 +3,9 @@ import b58 from 'bs58check';
 import { AbstractHDElectrumWallet } from './abstract-hd-electrum-wallet';
 import * as bitcoin from 'bitcoinjs-lib';
 import * as HDNode from 'bip32';
+import { ECPairFactory } from 'ecpair';
+const ecc = require('tiny-secp256k1');
+const ECPair = ECPairFactory(ecc);
 
 /**
  * HD Wallet (BIP39).
@@ -17,7 +20,7 @@ export class HDSegwitP2SHWallet extends AbstractHDElectrumWallet {
     return true;
   }
 
-  allowSendMax(): boolean {
+  allowSendMax() {
     return true;
   }
 
@@ -36,7 +39,7 @@ export class HDSegwitP2SHWallet extends AbstractHDElectrumWallet {
     const path = `m/49'/0'/0'/${internal ? 1 : 0}/${index}`;
     const child = root.derivePath(path);
 
-    return bitcoin.ECPair.fromPrivateKey(child.privateKey).toWIF();
+    return ECPair.fromPrivateKey(child.privateKey).toWIF();
   }
 
   _getExternalAddressByIndex(index) {

src/wallet/legacy-wallet.js

@@ -5,6 +5,9 @@ import * as bitcoin from 'bitcoinjs-lib';
 import * as BlueElectrum from '../BlueElectrum';
 import coinSelectAccumulative from 'coinselect/accumulative';
 import coinSelectSplit from 'coinselect/split';
+import { ECPairFactory } from 'ecpair';
+const ecc = require('tiny-secp256k1');
+const ECPair = ECPairFactory(ecc);
 
 /**
  *  Has private key and single address like "1ABCD....."
@@ -44,7 +47,7 @@ export class LegacyWallet extends AbstractWallet {
 
   async generate() {
     const buf = await randomBytes(32);
-    this.secret = bitcoin.ECPair.makeRandom({ rng: () => buf }).toWIF();
+    this.secret = ECPair.makeRandom({ rng: () => buf }).toWIF();
   }
 
   async generateFromEntropy(user) {
@@ -54,7 +57,7 @@ export class LegacyWallet extends AbstractWallet {
       const random = await randomBytes(user.length < 32 ? 32 - user.length : 0);
       const buf = Buffer.concat([user, random], 32);
       try {
-        this.secret = bitcoin.ECPair.fromPrivateKey(buf).toWIF();
+        this.secret = ECPair.fromPrivateKey(buf).toWIF();
         return;
       } catch (e) {
         if (i === 5) throw e;
@@ -70,7 +73,7 @@ export class LegacyWallet extends AbstractWallet {
     if (this._address) return this._address;
     let address;
     try {
-      const keyPair = bitcoin.ECPair.fromWIF(this.secret);
+      const keyPair = ECPair.fromWIF(this.secret);
       address = bitcoin.payments.p2pkh({
         pubkey: keyPair.publicKey,
       }).address;
@@ -314,7 +317,7 @@ export class LegacyWallet extends AbstractWallet {
     inputs.forEach(input => {
       if (!skipSigning) {
         // skiping signing related stuff
-        keyPair = bitcoin.ECPair.fromWIF(this.secret); // secret is WIF
+        keyPair = ECPair.fromWIF(this.secret); // secret is WIF
       }
       values[c] = input.value;
       c++;
@@ -372,12 +375,25 @@ export class LegacyWallet extends AbstractWallet {
   /**
    * Validates any address, including legacy, p2sh and bech32
    *
+   * p2tr addresses have extra logic, rejecting all versions >1
+   * @see https://github.com/BlueWallet/BlueWallet/issues/3394
+   * @see https://github.com/bitcoinjs/bitcoinjs-lib/issues/1750
+   * @see https://github.com/bitcoin/bips/blob/edffe529056f6dfd33d8f716fb871467c3c09263/bip-0350.mediawiki#Addresses_for_segregated_witness_outputs
+   *
    * @param address
    * @returns {boolean}
    */
   isAddressValid(address) {
     try {
-      bitcoin.address.toOutputScript(address);
+      bitcoin.address.toOutputScript(address); // throws, no?
+
+      if (!address.toLowerCase().startsWith('bc1')) return true;
+      const decoded = bitcoin.address.fromBech32(address);
+      if (decoded.version === 0) return true;
+      if (decoded.version === 1 && decoded.data.length !== 32) return false;
+      if (decoded.version === 1 && !ecc.isPoint(Buffer.concat([Buffer.from([2]), decoded.data]))) return false;
+      if (decoded.version > 1) return false;
+      // ^^^ some day, when versions above 1 will be actually utilized, we would need to unhardcode this
       return true;
     } catch (e) {
       return false;