diff --git a/bitcoin_client_js/package.json b/bitcoin_client_js/package.json index 2099ada4c..f09a7cf6c 100644 --- a/bitcoin_client_js/package.json +++ b/bitcoin_client_js/package.json @@ -29,9 +29,11 @@ "node": ">=14" }, "dependencies": { + "@bitcoinerlab/descriptors": "^1.0.2", + "@bitcoinerlab/secp256k1": "^1.0.5", "@ledgerhq/hw-transport": "^6.20.0", "bip32-path": "^0.4.2", - "bitcoinjs-lib": "^6.0.1" + "bitcoinjs-lib": "^6.1.3" }, "devDependencies": { "@ledgerhq/hw-transport-node-speculos-http": "^6.24.1", diff --git a/bitcoin_client_js/src/__tests__/appClient.test.ts b/bitcoin_client_js/src/__tests__/appClient.test.ts index 191b38509..457ad902b 100644 --- a/bitcoin_client_js/src/__tests__/appClient.test.ts +++ b/bitcoin_client_js/src/__tests__/appClient.test.ts @@ -260,6 +260,36 @@ describe("test AppClient", () => { expect(walletHmac.length).toEqual(32); }); + //https://wizardsardine.com/blog/ledger-vulnerability-disclosure/ + it('can generate a correct address or throw on a:X', async () => { + try { + const walletPolicy = new WalletPolicy('Fixed Vulnerability', 'wsh(and_b(pk(@0/**),a:1))', [ + "[f5acc2fd/84'/1'/0']tpubDCtKfsNyRhULjZ9XMS4VKKtVcPdVDi8MKUbcSD9MJDyjRu1A2ND5MiipozyyspBT9bg8upEp7a8EAgFxNxXn1d7QkdbL52Ty5jiSLcxPt1P" + ]); + + const automation = JSON.parse(fs.readFileSync('src/__tests__/automations/register_wallet_accept.json').toString()); + await setSpeculosAutomation(transport, automation); + + const [walletId, walletHmac] = await app.registerWallet(walletPolicy); + + expect(walletId).toEqual(walletPolicy.getId()); + expect(walletHmac.length).toEqual(32); + + const address = await app.getWalletAddress( + walletPolicy, + walletHmac, + 0, + 0, + false + ); + //version > 2.1.1 + expect(address).toEqual('tb1q5lyn9807ygs7pc52980mdeuwl9wrq5c8n3kntlhy088h6fqw4gzspw9t9m'); + } catch (error) { + //version <= 2.1.1 + expect(error.message).toMatch(/^Third party address validation mismatch/); + } + }); + it("can register a miniscript wallet", async () => { const walletPolicy = new WalletPolicy( "Decaying 3-of-3", @@ -417,4 +447,4 @@ describe("test AppClient", () => { const result = await app.signMessage(Buffer.from(msg, "ascii"), path) expect(result).toEqual("H4frM6TYm5ty1MAf9o/Zz9Qiy3VEldAYFY91SJ/5nYMAZY1UUB97fiRjKW8mJit2+V4OCa1YCqjDqyFnD9Fw75k="); }); -}); \ No newline at end of file +}); diff --git a/bitcoin_client_js/src/lib/appClient.ts b/bitcoin_client_js/src/lib/appClient.ts index ddfa169b6..172154af4 100644 --- a/bitcoin_client_js/src/lib/appClient.ts +++ b/bitcoin_client_js/src/lib/appClient.ts @@ -1,4 +1,8 @@ +import * as descriptors from '@bitcoinerlab/descriptors'; +import * as secp256k1 from '@bitcoinerlab/secp256k1'; +const { Descriptor } = descriptors.DescriptorsFactory(secp256k1); import Transport from '@ledgerhq/hw-transport'; +import { networks } from 'bitcoinjs-lib'; import { pathElementsToBuffer, pathStringToArray } from './bip32'; import { ClientCommandInterpreter } from './clientCommands'; @@ -184,8 +188,6 @@ export class AppClient { walletPolicy: WalletPolicy ): Promise { - await this.validatePolicy(walletPolicy); - const clientInterpreter = new ClientCommandInterpreter(); clientInterpreter.addKnownWalletPolicy(walletPolicy); @@ -236,8 +238,6 @@ export class AppClient { throw new Error('Invalid HMAC length'); } - await this.validatePolicy(walletPolicy); - const clientInterpreter = new ClientCommandInterpreter(); clientInterpreter.addKnownWalletPolicy(walletPolicy); @@ -257,7 +257,9 @@ export class AppClient { clientInterpreter ); - return response.toString('ascii'); + const address = response.toString('ascii'); + await this.validateAddress(address, walletPolicy, change, addressIndex); + return address; } /** @@ -279,7 +281,6 @@ export class AppClient { walletHMAC: Buffer | null, progressCallback?: () => void ): Promise<[number, PartialSignature][]> { - await this.validatePolicy(walletPolicy); if (typeof psbt === 'string') { psbt = Buffer.from(psbt, "base64"); @@ -402,12 +403,69 @@ export class AppClient { return result.toString('base64'); } + /* Performs any additional check on the genetated address before returning it.*/ + private async validateAddress( + address: string, + walletPolicy: WalletPolicy, + change: number, + addressIndex: number + ) { + if (change !== 0 && change !== 1) + throw new Error('Change can only be 0 or 1'); + if (addressIndex < 0 || !Number.isInteger(addressIndex)) + throw new Error('Invalid address index'); + const appAndVer = await this.getAppAndVersion(); + let network; + if (appAndVer.name === 'Bitcoin Test') { + network = networks.testnet; + } else if (appAndVer.name === 'Bitcoin') { + network = networks.bitcoin; + } else { + throw new Error( + `Invalid network: ${appAndVer.name}. Expected 'Bitcoin Test' or 'Bitcoin'.` + ); + } + let expression = walletPolicy.descriptorTemplate; + for (let i = 0; i < walletPolicy.keys.length; i++) { + const keyPath = walletPolicy.keys[i] + '/' + change + '/' + addressIndex; + expression = expression + .replace('@' + i + '/**', keyPath) + .replace('@' + i + '/<0;1>', keyPath); + } + let thirdPartyValidationApplicable = true; + let thirdPartyGeneratedAddress: string; + try { + thirdPartyGeneratedAddress = new Descriptor({ + expression, + network + }).getAddress(); + } catch(err) { + // Note: @bitcoinerlab/descriptors@1.0.x does not support Tapscript yet. + // These are the supported descriptors: + // - pkh(KEY) + // - wpkh(KEY) + // - sh(wpkh(KEY)) + // - sh(SCRIPT) + // - wsh(SCRIPT) + // - sh(wsh(SCRIPT)), where + // SCRIPT is any of the (non-tapscript) fragments in: https://bitcoin.sipa.be/miniscript/ + // + // Other expressions are not supported and third party validation would not be applicable: + thirdPartyValidationApplicable = false; + } + if (thirdPartyValidationApplicable) { + if (address !== thirdPartyGeneratedAddress) { + throw new Error( + `Third party address validation mismatch: ${address} != ${thirdPartyGeneratedAddress}` + ); + } + } else { + await this.validatePolicy(walletPolicy); + } + } + /* Performs any additional checks on the policy before using it.*/ private async validatePolicy(walletPolicy: WalletPolicy) { - // TODO: Once an independent implementation of miniscript in JavaScript is available, - // we will replace the checks in this section with a generic comparison between the - // address produced by the app and the one computed locally (like the python and Rust - // clients). Until then, we filter for any known bug. let appAndVer = undefined;