Skip to content

Make the transport proxy throw exceptions on every sw except 0x9000 (… #571

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
chcmedeiros merged 1 commit into from
Aug 6, 2025
Merged

Make the transport proxy throw exceptions on every sw except 0x9000 (… #571

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
"@grpc/grpc-js": "^1.13.4",
"@grpc/proto-loader": "^0.7.15",
"@ledgerhq/hw-transport-http": "^6.30.8",
"@ledgerhq/errors": "^6.23.0",
"axios": "^1.11.0",
"axios-retry": "^4.5.0",
"dockerode": "^4.0.7",
Expand Down
12 changes: 5 additions & 7 deletions pnpm-lock.yaml
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions src/Zemu.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
******************************************************************************* */

import { resolve } from 'node:path'
import { TransportStatusError } from '@ledgerhq/errors'
import type Transport from '@ledgerhq/hw-transport'
import HttpTransport from '@ledgerhq/hw-transport-http'
import axios, { type AxiosResponse } from 'axios'
Expand Down Expand Up @@ -66,6 +67,10 @@ import {
} from './types'
import { isTouchDevice, zondaxToggleBlindSigning, zondaxToggleExpertMode, zondaxTouchEnableSpecialMode } from './zondax'

enum ApduError {
NoError = 0x9000,
}

export default class Zemu {
public startOptions!: IStartOptions

Expand Down Expand Up @@ -458,6 +463,11 @@ export default class Zemu {
try {
self.lastTransportError = null // Clear previous error
const result = await target.send(cla, ins, p1, p2, data, statusList)
const sw = result.readUInt16BE(result.length - 2)

if (sw !== ApduError.NoError) {
throw new TransportStatusError(sw)
}
Comment on lines +466 to +470
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot Aug 6, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Status word validation logic is correct but breaks existing transport behavior.

The implementation correctly extracts the 2-byte status word from the end of the APDU response and throws TransportStatusError for non-success codes. However, this fundamentally changes the transport behavior by ignoring the statusList parameter that was previously used to accept specific error codes.

The current implementation ignores the statusList parameter and throws exceptions for ANY non-0x9000 status word. This breaks backward compatibility for code that intentionally accepts specific error status codes.

Consider this alternative approach:

-              const sw = result.readUInt16BE(result.length - 2)
-
-              if (sw !== ApduError.NoError) {
-                throw new TransportStatusError(sw)
-              }
+              const sw = result.readUInt16BE(result.length - 2)
+
+              // Only throw if status word is not in the accepted list
+              if (sw !== ApduError.NoError && (!statusList || !statusList.includes(sw))) {
+                throw new TransportStatusError(sw)
+              }

This preserves the original behavior where specific error codes can be accepted via the statusList parameter.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const sw = result.readUInt16BE(result.length - 2)
if (sw !== ApduError.NoError) {
throw new TransportStatusError(sw)
}
const sw = result.readUInt16BE(result.length - 2)
// Only throw if status word is not in the accepted list
if (sw !== ApduError.NoError && (!statusList || !statusList.includes(sw))) {
throw new TransportStatusError(sw)
}
🤖 Prompt for AI Agents 
In src/Zemu.ts around lines 466 to 470, the current code throws a
TransportStatusError for any status word not equal to ApduError.NoError,
ignoring the statusList parameter. To fix this, modify the logic to check if the
extracted status word is included in the statusList array before throwing an
error. If the status word is not in statusList, then throw the
TransportStatusError; otherwise, allow the response to pass. This preserves
backward compatibility by respecting the statusList parameter.

return result
} catch (error) {
// Store the error for later checks
Expand Down
5 changes: 4 additions & 1 deletion tests/error-handling.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,16 @@ describe('Error Handling', () => {
const validINS = 0x00
const p1 = 0x00
const p2 = 0x00
const data = Buffer.from([])
// Provide a statusList to make sure zemu throws an exception even on accepted status words
const statusList = [0x9000, 0x6e00]

// Start timer to measure how long the error takes
const startTime = Date.now()

try {
// This should fail with CLA_NOT_SUPPORTED (0x6E00)
await transport.send(invalidCLA, validINS, p1, p2)
await transport.send(invalidCLA, validINS, p1, p2, data, statusList)

// If we get here, the test failed - we expected an error
expect.fail('Expected transport.send to throw an error')
Expand Down
Loading