From e00bc1e53abdb68d01f7bb2fa6d8645305af05be Mon Sep 17 00:00:00 2001 From: Luis Rojas Date: Mon, 6 Apr 2026 18:38:31 -0600 Subject: [PATCH] fix(express): prevent double-stringify of string payloads in handleV2OFCSignPayload The handler unconditionally called JSON.stringify(payload), which double-stringifies when the io-ts Json codec resolves the input as a string (the common case for tx/build responses). This caused tx/send to reject with 400 invalid signature. Apply the same typeof guard already used in the ext-signing-mode handler. TICKET: CAAS-1186 --- modules/express/src/clientRoutes.ts | 2 +- .../test/unit/clientRoutes/signPayload.ts | 26 +++++++++ .../test/unit/typedRoutes/ofcSignPayload.ts | 54 +++++++++++++++++++ 3 files changed, 81 insertions(+), 1 deletion(-) diff --git a/modules/express/src/clientRoutes.ts b/modules/express/src/clientRoutes.ts index 8e6ee42e63..22e1b0a769 100755 --- a/modules/express/src/clientRoutes.ts +++ b/modules/express/src/clientRoutes.ts @@ -625,7 +625,7 @@ export async function handleV2OFCSignPayload( const walletPassphrase = bodyWalletPassphrase || getWalletPwFromEnv(wallet.id()); const tradingAccount = wallet.toTradingAccount(); - const stringifiedPayload = JSON.stringify(payload); + const stringifiedPayload = typeof payload === 'string' ? payload : JSON.stringify(payload); const signature = await tradingAccount.signPayload({ payload: stringifiedPayload, walletPassphrase, diff --git a/modules/express/test/unit/clientRoutes/signPayload.ts b/modules/express/test/unit/clientRoutes/signPayload.ts index b239ff537a..adcfee820e 100644 --- a/modules/express/test/unit/clientRoutes/signPayload.ts +++ b/modules/express/test/unit/clientRoutes/signPayload.ts @@ -87,6 +87,32 @@ describe('Sign an arbitrary payload with trading account key', function () { throw new Error(`Response did not match expected codec`); }); }); + it('should not double-stringify when decoded payload is already a string', async function () { + // When the io-ts Json codec matches a string input (left-to-right union resolution), + // decoded.payload is the original string. The handler must not JSON.stringify it again. + const expectedResponse = { + payload: stringifiedPayload, + signature, + }; + const req = { + bitgo: bitGoStub, + body: { + payload: stringifiedPayload, + walletId, + }, + decoded: { + walletId, + payload: stringifiedPayload, + }, + query: {}, + } as unknown as ExpressApiRouteRequest<'express.ofc.signPayload', 'post'>; + const result = await handleV2OFCSignPayload(req).should.be.resolvedWith(expectedResponse); + result.payload.should.equal(stringifiedPayload); + result.payload.should.not.startWith('"'); + decodeOrElse('OfcSignPayloadResponse200', OfcSignPayloadResponse[200], result, (_) => { + throw new Error(`Response did not match expected codec`); + }); + }); it('should decode handler response with OfcSignPayloadResponse codec', async function () { const expected = { payload: JSON.stringify(payload), diff --git a/modules/express/test/unit/typedRoutes/ofcSignPayload.ts b/modules/express/test/unit/typedRoutes/ofcSignPayload.ts index 2248b3e5b7..59cdd54f4d 100644 --- a/modules/express/test/unit/typedRoutes/ofcSignPayload.ts +++ b/modules/express/test/unit/typedRoutes/ofcSignPayload.ts @@ -136,6 +136,60 @@ describe('OfcSignPayload codec tests', function () { assert.ok(decodedResponse); }); + it('should not double-stringify a stringified JSON payload', async function () { + const originalPayload = '{"coin":"ofctbtc","recipients":[{"address":"abc123","amount":"1000"}]}'; + const requestBody = { + walletId: 'ofc-wallet-id-123', + payload: originalPayload, + walletPassphrase: 'test_passphrase', + }; + + const mockTradingAccount = { + signPayload: sinon.stub().resolves(mockSignPayloadResponse.signature), + }; + + const mockWallet = { + id: () => requestBody.walletId, + toTradingAccount: sinon.stub().returns(mockTradingAccount), + }; + + const walletsGetStub = sinon.stub().resolves(mockWallet); + const mockWallets = { get: walletsGetStub }; + const mockCoin = { wallets: sinon.stub().returns(mockWallets) }; + sinon.stub(BitGo.prototype, 'coin').returns(mockCoin as any); + + const result = await agent + .post('/api/v2/ofc/signPayload') + .set('Authorization', 'Bearer test_access_token_12345') + .set('Content-Type', 'application/json') + .send(requestBody); + + assert.strictEqual(result.status, 200); + const decodedResponse = assertDecode(OfcSignPayloadResponse200, result.body); + + // The returned payload must not be double-stringified. + // A double-stringified payload would start with a quote character and contain escaped quotes. + assert.strictEqual( + decodedResponse.payload.startsWith('"'), + false, + 'payload was double-stringified: starts with a quote character' + ); + assert.strictEqual( + decodedResponse.payload.includes('\\"'), + false, + 'payload was double-stringified: contains escaped quotes' + ); + + // The payload passed to tradingAccount.signPayload must match the original string + const signCall = mockTradingAccount.signPayload.getCall(0); + assert.ok(signCall, 'tradingAccount.signPayload should have been called'); + assert.strictEqual( + signCall.args[0].payload, + originalPayload, + 'tradingAccount.signPayload received a different payload than the original' + ); + }); + it('should successfully sign payload without walletPassphrase (uses env)', async function () { const requestBody = { walletId: 'ofc-wallet-id-123',