fix: ledger fall back to blind signing when clear-sign fails

packages/keyring-eth-ledger-bridge/CHANGELOG.md

@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
 
 - Bump `@metamask/keyring-sdk` from `^2.0.2` to `^2.1.1` ([#544](https://github.com/MetaMask/accounts/pull/544), [#546](https://github.com/MetaMask/accounts/pull/546))
 
+### Fixed
+
+- Fall back to blind signing on mobile when `clearSignTransaction` fails, except when the user rejects on the device ([#522](https://github.com/MetaMask/accounts/pull/522))
+
 ## [12.0.2]
 
 ### Changed

packages/keyring-eth-ledger-bridge/package.json

@@ -72,6 +72,7 @@
     "@ethereumjs/tx": "^5.4.0",
     "@ethereumjs/util": "^9.1.0",
     "@ledgerhq/hw-app-eth": "^6.42.0",
+    "@ledgerhq/hw-transport": "^6.31.3",
     "@metamask/eth-sig-util": "^8.2.0",
     "@metamask/hw-wallet-sdk": "^0.8.0",
     "@metamask/keyring-api": "^23.1.0",
@@ -82,7 +83,6 @@
     "@ethereumjs/common": "^4.4.0",
     "@lavamoat/allow-scripts": "^3.2.1",
     "@lavamoat/preinstall-always-fail": "^2.1.0",
-    "@ledgerhq/hw-transport": "^6.31.3",
     "@ledgerhq/types-cryptoassets": "^7.15.1",
     "@ledgerhq/types-devices": "^6.25.3",
     "@ledgerhq/types-live": "^6.52.0",

packages/keyring-eth-ledger-bridge/src/ledger-mobile-bridge.test.ts

@@ -1,7 +1,7 @@
 import { Common, Chain, Hardfork } from '@ethereumjs/common';
 import { TransactionFactory } from '@ethereumjs/tx';
 import { bytesToHex } from '@ethereumjs/util';
-import Transport from '@ledgerhq/hw-transport';
+import Transport, { TransportStatusError } from '@ledgerhq/hw-transport';
 import { EIP712Message } from '@ledgerhq/types-live';
 import { remove0x } from '@metamask/utils';
 
@@ -25,6 +25,7 @@ describe('LedgerMobileBridge', function () {
   const mockEthApp = {
     signEIP712Message: jest.fn(),
     clearSignTransaction: jest.fn(),
+    signTransaction: jest.fn(),
     getAddress: jest.fn(),
     signPersonalMessage: jest.fn(),
     openEthApp: jest.fn(),
@@ -248,6 +249,56 @@ describe('LedgerMobileBridge', function () {
         nft: false,
       });
     });
+
+    it('falls back to blind signing when clear-sign fails for unsupported resolution', async function () {
+      const hdPath = "m/44'/60'/0'/0/0";
+      const tx =
+        'f86d8202b38477359400825208944592d8f8d7b001e72cb26a73e4fa1806a51ac79d880de0b6b3a7640000802ba0699ff162205967ccbabae13e07cdd4284258d46ec1051a70a51be51ec2bc69f3a04e6944d508244ea54a62ebf9a72683eeadacb73ad7c373ee542f1998147b220e';
+      const blindSignResult = {
+        v: '2b',
+        r: '0699ff162205967ccbabae13e07cdd4284258d46ec1051a70a51be51ec2bc69f3',
+        s: '04e6944d508244ea54a62ebf9a72683eeadacb73ad7c373ee542f1998147b220e',
+      };
+      mockEthApp.clearSignTransaction.mockRejectedValueOnce(
+        new Error('resolution failed'),
+      );
+      mockEthApp.signTransaction.mockResolvedValueOnce(blindSignResult);
+
+      const consoleSpy = jest.spyOn(console, 'warn').mockImplementation();
+
+      const result = await bridge.deviceSignTransaction({
+        hdPath,
+        tx,
+      });
+
+      expect(result).toStrictEqual(blindSignResult);
+      expect(mockEthApp.clearSignTransaction).toHaveBeenCalledTimes(1);
+      expect(mockEthApp.signTransaction).toHaveBeenCalledTimes(1);
+      expect(mockEthApp.signTransaction).toHaveBeenCalledWith(hdPath, tx, null);
+      expect(consoleSpy).toHaveBeenCalledTimes(1);
+      expect(consoleSpy.mock.calls[0]?.[0]).toBe(
+        'Ledger clear-sign failed; falling back to blind signing.',
+      );
+
+      consoleSpy.mockRestore();
+    });
+
+    it('does not fall back when the user rejects on the device', async function () {
+      const hdPath = "m/44'/60'/0'/0/0";
+      const tx =
+        'f86d8202b38477359400825208944592d8f8d7b001e72cb26a73e4fa1806a51ac79d880de0b6b3a7640000802ba0699ff162205967ccbabae13e07cdd4284258d46ec1051a70a51be51ec2bc69f3a04e6944d508244ea54a62ebf9a72683eeadacb73ad7c373ee542f1998147b220e';
+      const rejection = new TransportStatusError(0x6985);
+      mockEthApp.clearSignTransaction.mockRejectedValueOnce(rejection);
+
+      await expect(
+        bridge.deviceSignTransaction({
+          hdPath,
+          tx,
+        }),
+      ).rejects.toThrow(rejection);
+
+      expect(mockEthApp.signTransaction).not.toHaveBeenCalled();
+    });
   });
 
   describe('deviceSignTypedData', function () {

packages/keyring-eth-ledger-bridge/src/ledger-mobile-bridge.ts

@@ -1,3 +1,4 @@
+import { TransportStatusError } from '@ledgerhq/hw-transport';
 import type Transport from '@ledgerhq/hw-transport';
 
 import { ERC20_WRITE_SELECTORS, NFT_ONLY_SELECTORS } from './constants';
@@ -19,6 +20,9 @@ import { TransportMiddleware } from './ledger-transport-middleware';
 import { LedgerMobileBridgeOptions } from './type';
 import { getTransactionSelector } from './utils';
 
+/** Ledger APDU: CONDITIONS_OF_USE_NOT_SATISFIED (user rejected on device). */
+const LEDGER_USER_REJECTION_STATUS = 0x6985;
+
 // MobileBridge Type will always use LedgerBridge with LedgerMobileBridgeOptions
 export type MobileBridge = LedgerBridge<LedgerMobileBridgeOptions> & {
   openEthApp(): Promise<void>;
@@ -102,6 +106,9 @@ export class LedgerMobileBridge implements MobileBridge {
    * Method to sign a transaction
    * Sending the hexadecimal transaction message to the device and returning the signed transaction.
    *
+   * Tries clear-signing first. If resolution fails (e.g. no Ledger plugin for the chain or contract),
+   * falls back to blind signing via `signTransaction(..., null)` unless the user rejected on the device.
+   *
    * @param params - An object contains tx, hdPath.
    * @param params.tx - The raw ethereum transaction in hexadecimal to sign.
    * @param params.hdPath - The BIP 32 path of the account.
@@ -115,11 +122,25 @@ export class LedgerMobileBridge implements MobileBridge {
     const nft = Boolean(selector && NFT_ONLY_SELECTORS.has(selector));
     const erc20 = Boolean(selector && ERC20_WRITE_SELECTORS.has(selector));
 
-    return this.#getEthApp().clearSignTransaction(hdPath, tx, {
-      externalPlugins: true,
-      erc20,
-      nft,
-    });
+    const ethApp = this.#getEthApp();
+
+    try {
+      return await ethApp.clearSignTransaction(hdPath, tx, {
+        externalPlugins: true,
+        erc20,
+        nft,
+      });
+    } catch (error: unknown) {
+      if (LedgerMobileBridge.#isLedgerUserRejection(error)) {
+        throw error;
+      }
+
+      console.warn(
+        'Ledger clear-sign failed; falling back to blind signing.',
+        error,
+      );
+      return ethApp.signTransaction(hdPath, tx, null);
+    }
   }
 
   /**
@@ -235,4 +256,18 @@ export class LedgerMobileBridge implements MobileBridge {
   #getEthApp(): MetaMaskLedgerHwAppEth {
     return this.#getTransportMiddleWare().getEthApp();
   }
+
+  /**
+   * Detects an explicit on-device rejection so we do not fall back to blind signing
+   * and re-prompt the user.
+   *
+   * @param error - Error from Ledger transport or hw-app-eth.
+   * @returns True when the user rejected the action on the device.
+   */
+  static #isLedgerUserRejection(error: unknown): boolean {
+    return (
+      error instanceof TransportStatusError &&
+      error.statusCode === LEDGER_USER_REJECTION_STATUS
+    );
+  }
 }