diff --git a/bitcoin_client_js/package.json b/bitcoin_client_js/package.json index f09a7cf6c..362df99f0 100644 --- a/bitcoin_client_js/package.json +++ b/bitcoin_client_js/package.json @@ -1,6 +1,6 @@ { "name": "ledger-bitcoin", - "version": "0.2.2", + "version": "0.2.3", "description": "Ledger Hardware Wallet Bitcoin Application Client", "main": "build/main/index.js", "typings": "build/main/index.d.ts", diff --git a/bitcoin_client_js/src/__tests__/appClient.test.ts b/bitcoin_client_js/src/__tests__/appClient.test.ts index 457ad902b..a0b803a6f 100644 --- a/bitcoin_client_js/src/__tests__/appClient.test.ts +++ b/bitcoin_client_js/src/__tests__/appClient.test.ts @@ -262,31 +262,46 @@ describe("test AppClient", () => { //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/); + for (const template of [ + 'wsh(and_b(pk(@0/**),a:1))', + 'wsh(and_b(pk(@0/<0;1>/*),a:1))' + ]) { + try { + const walletPolicy = new WalletPolicy('Fixed Vulnerability', template, [ + "[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/ + ); + } } }); @@ -297,6 +312,7 @@ describe("test AppClient", () => { [ "[76223a6e/48'/1'/0'/2']tpubDE7NQymr4AFtewpAsWtnreyq9ghkzQBXpCZjWLFVRAvnbf7vya2eMTvT2fPapNqL8SuVvLQdbUbMfWLVDCZKnsEBqp6UK93QEzL8Ck23AwF", "[f5acc2fd/48'/1'/0'/2']tpubDFAqEGNyad35aBCKUAXbQGDjdVhNueno5ZZVEn3sQbW5ci457gLR7HyTmHBg93oourBssgUxuWz1jX5uhc1qaqFo9VsybY1J5FuedLfm4dK", + "tpubDCoDDpHR1MYXcFrarTcwBufQvWPXSSZpGxjnhRaW612TMxs5TWDEPdbYRHtQdZ9z1UqtKGQKVQ4FqejzbFSdvQvJsD75yrgh7thVoFho6jE" ] ); diff --git a/bitcoin_client_js/src/lib/appClient.ts b/bitcoin_client_js/src/lib/appClient.ts index 172154af4..763d9bbef 100644 --- a/bitcoin_client_js/src/lib/appClient.ts +++ b/bitcoin_client_js/src/lib/appClient.ts @@ -66,14 +66,6 @@ function makePartialSignature(pubkeyAugm: Buffer, signature: Buffer): PartialSig } } -/** - * Checks whether a descriptor template contains an `a:` fragment. - */ -function containsA(descriptorTemplate: string): boolean { - const matches = descriptorTemplate.match(/[asctdvjnlu]+:/g) || []; - return matches.some(match => match.includes('a')); -} - /** * This class encapsulates the APDU protocol documented at * https://github.com/LedgerHQ/app-bitcoin-new/blob/master/doc/bitcoin.md @@ -207,8 +199,20 @@ export class AppClient { `Invalid response length. Expected 64 bytes, got ${response.length}` ); } + const walletId = response.subarray(0, 32); + const walletHMAC = response.subarray(32); + + // sanity check: derive and validate the first address with a 3rd party + const firstAddrDevice = await this.getWalletAddress( + walletPolicy, + walletHMAC, + 0, + 0, + false + ); + await this.validateAddress(firstAddrDevice, walletPolicy, 0, 0); - return [response.subarray(0, 32), response.subarray(32)]; + return [walletId, walletHMAC]; } /** @@ -412,8 +416,9 @@ export class AppClient { ) { if (change !== 0 && change !== 1) throw new Error('Change can only be 0 or 1'); + const isChange: boolean = change === 1; if (addressIndex < 0 || !Number.isInteger(addressIndex)) - throw new Error('Invalid address index'); + throw new Error('Invalid address index'); const appAndVer = await this.getAppAndVersion(); let network; if (appAndVer.name === 'Bitcoin Test') { @@ -426,12 +431,22 @@ export class AppClient { ); } 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); + // Replace change: + expression = expression.replace(/\/\*\*/g, `/<0;1>/*`); + const regExpMN = new RegExp(`/<(\\d+);(\\d+)>`, 'g'); + let matchMN; + while ((matchMN = regExpMN.exec(expression)) !== null) { + const [M, N] = [parseInt(matchMN[1], 10), parseInt(matchMN[2], 10)]; + expression = expression.replace(`/<${M};${N}>`, `/${isChange ? N : M}`); } + // Replace index: + expression = expression.replace(/\/\*/g, `/${addressIndex}`); + // Replace origin: + for (let i = 0; i < walletPolicy.keys.length; i++) + expression = expression.replace( + new RegExp(`@${i}`, 'g'), + walletPolicy.keys[i] + ); let thirdPartyValidationApplicable = true; let thirdPartyGeneratedAddress: string; try { @@ -439,7 +454,7 @@ export class AppClient { expression, network }).getAddress(); - } catch(err) { + } catch (err) { // Note: @bitcoinerlab/descriptors@1.0.x does not support Tapscript yet. // These are the supported descriptors: // - pkh(KEY) @@ -453,41 +468,13 @@ export class AppClient { // 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) { - - let appAndVer = undefined; - - if (containsA(walletPolicy.descriptorTemplate)) { - appAndVer = appAndVer || await this.getAppAndVersion(); - if (["2.1.0", "2.1.1"].includes(appAndVer.version)) { - // Versions 2.1.0 and 2.1.1 produced incorrect scripts for policies containing - // the `a:` fragment. - throw new Error("Please update your Ledger Bitcoin app.") - } - } - - if (walletPolicy.descriptorTemplate.includes("thresh(1,")) { - appAndVer = appAndVer || await this.getAppAndVersion(); - if (["2.1.0", "2.1.1", "2.1.2"].includes(appAndVer.version)) { - // Versions 2.1.0 and 2.1.1 and "2.1.2" produced incorrect scripts for policies - // containing an unusual thresh fragment with k = n = 1, that is "thresh(1,X)". - // (The check above has false positives, as it also matches "thresh" fragments - // where n > 1; however, better to be overzealous). - throw new Error("Please update your Ledger Bitcoin app.") - } - } + if ( + thirdPartyValidationApplicable && + address !== thirdPartyGeneratedAddress + ) + throw new Error( + `Third party address validation mismatch: ${address} != ${thirdPartyGeneratedAddress}` + ); } }