Skip to content

Commit b8d5d66

Browse files
authored
Validate contract ID on transaction deserialization (#1349)
* AssembledTransaction: fix fromXDR and fromJSON methods * Addressed Copilot feedback * Refactor * Copilot feedback * Updated changelog
1 parent 2f52d0e commit b8d5d66

File tree

4 files changed

+274
-14
lines changed

4 files changed

+274
-14
lines changed

.gitignore

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,3 +17,6 @@ target
1717
.soroban
1818

1919
config/tsconfig.tmp.json
20+
21+
.claude/
22+
.copilot/

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ A breaking change will get clearly marked in this log.
88

99
### Fixed
1010
* Sanitize identifiers and escape string literals in generated TypeScript bindings to prevent code injection via malicious contract spec names. `sanitizeIdentifier` now strips non-identifier characters, and a new `escapeStringLiteral` helper escapes quotes and newlines in string contexts ([#1345](https://github.com/stellar/js-stellar-sdk/pull/1345)).
11+
* `AssembledTransaction.fromXDR()` and `fromJSON()` now validate that the deserialized transaction targets the expected contract, rejecting mismatched contract IDs and non-invokeContract operations. ([#1349](https://github.com/stellar/js-stellar-sdk/pull/1349)).
1112

1213
## [v14.6.1](https://github.com/stellar/js-stellar-sdk/compare/v14.6.0...v14.6.1)
1314

src/contract/assembled_transaction.ts

Lines changed: 80 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -361,6 +361,66 @@ export class AssembledTransaction<T> {
361361
});
362362
}
363363

364+
/**
365+
* Validate that a built transaction is a single invokeContract operation
366+
* targeting the expected contract, and return the parsed InvokeContractArgs.
367+
*/
368+
private static validateInvokeContractOp(
369+
built: Tx,
370+
expectedContractId: string,
371+
): xdr.InvokeContractArgs {
372+
if (built.operations.length !== 1) {
373+
throw new Error(
374+
"Transaction envelope must contain exactly one operation.",
375+
);
376+
}
377+
378+
const operation = built.operations[0];
379+
380+
if (operation.type !== "invokeHostFunction") {
381+
throw new Error(
382+
"Transaction envelope does not contain an invokeHostFunction operation.",
383+
);
384+
}
385+
386+
const invokeOp = operation as Operation.InvokeHostFunction;
387+
388+
if (invokeOp.func.switch().name !== "hostFunctionTypeInvokeContract") {
389+
throw new Error(
390+
"Transaction envelope does not contain an invokeContract host function.",
391+
);
392+
}
393+
394+
const invokeContractArgs = invokeOp.func.value() as xdr.InvokeContractArgs;
395+
396+
let contractAddress: xdr.ScAddress;
397+
let functionName: string;
398+
399+
try {
400+
contractAddress = invokeContractArgs.contractAddress();
401+
functionName = invokeContractArgs.functionName().toString("utf-8");
402+
} catch {
403+
throw new Error(
404+
"Could not extract contract address or method name from the transaction envelope.",
405+
);
406+
}
407+
408+
if (!contractAddress || !functionName) {
409+
throw new Error(
410+
"Could not extract contract address or method name from the transaction envelope.",
411+
);
412+
}
413+
414+
const xdrContractId = Address.fromScAddress(contractAddress).toString();
415+
if (xdrContractId !== expectedContractId) {
416+
throw new Error(
417+
`Transaction envelope targets contract ${xdrContractId}, but this Client is configured for ${expectedContractId}.`,
418+
);
419+
}
420+
421+
return invokeContractArgs;
422+
}
423+
364424
static fromJSON<T>(
365425
options: Omit<AssembledTransactionOptions<T>, "args">,
366426
{
@@ -378,6 +438,20 @@ export class AssembledTransaction<T> {
378438
): AssembledTransaction<T> {
379439
const txn = new AssembledTransaction(options);
380440
txn.built = TransactionBuilder.fromXDR(tx, options.networkPassphrase) as Tx;
441+
442+
const invokeContractArgs = AssembledTransaction.validateInvokeContractOp(
443+
txn.built,
444+
options.contractId,
445+
);
446+
447+
const xdrMethod = invokeContractArgs.functionName().toString("utf-8");
448+
449+
if (xdrMethod !== options.method) {
450+
throw new Error(
451+
`Transaction envelope calls method '${xdrMethod}', but the provided method is '${options.method}'.`,
452+
);
453+
}
454+
381455
txn.simulationResult = {
382456
auth: simulationResult.auth.map((a) =>
383457
xdr.SorobanAuthorizationEntry.fromXDR(a, "base64"),
@@ -419,18 +493,12 @@ export class AssembledTransaction<T> {
419493
envelope,
420494
options.networkPassphrase,
421495
) as Tx;
422-
const operation = built.operations[0] as Operation.InvokeHostFunction;
423-
if (!operation?.func?.value || typeof operation.func.value !== "function") {
424-
throw new Error(
425-
"Could not extract the method from the transaction envelope.",
426-
);
427-
}
428-
const invokeContractArgs = operation.func.value() as xdr.InvokeContractArgs;
429-
if (!invokeContractArgs?.functionName) {
430-
throw new Error(
431-
"Could not extract the method name from the transaction envelope.",
432-
);
433-
}
496+
497+
const invokeContractArgs = AssembledTransaction.validateInvokeContractOp(
498+
built,
499+
options.contractId,
500+
);
501+
434502
const method = invokeContractArgs.functionName().toString("utf-8");
435503
const txn = new AssembledTransaction({
436504
...options,

test/unit/server/soroban/assembled_transaction.test.ts

Lines changed: 190 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,18 @@ import { describe, it, beforeEach, afterEach, expect, vi } from "vitest";
33
import { serverUrl } from "../../../constants";
44
import { StellarSdk } from "../../../test-utils/stellar-sdk-import";
55

6-
const { Account, Keypair, rpc, contract, SorobanDataBuilder, xdr, Address } =
7-
StellarSdk;
6+
const {
7+
Account,
8+
Keypair,
9+
Operation,
10+
TransactionBuilder,
11+
TimeoutInfinite,
12+
rpc,
13+
contract,
14+
SorobanDataBuilder,
15+
xdr,
16+
Address,
17+
} = StellarSdk;
818
const { Server } = StellarSdk.rpc;
919

1020
const restoreTxnData = StellarSdk.SorobanDataBuilder.fromXDR(
@@ -155,3 +165,181 @@ describe("AssembledTransaction", () => {
155165
);
156166
});
157167
});
168+
169+
describe("Contract ID validation on deserialization", () => {
170+
const networkPassphrase = "Standalone Network ; February 2017";
171+
const keypair = Keypair.random();
172+
const source = new Account(keypair.publicKey(), "0");
173+
174+
const victimContractId =
175+
"CAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAD2KM";
176+
const attackerContractId =
177+
"CC53XO53XO53XO53XO53XO53XO53XO53XO53XO53XO53XO53XO53WQD5";
178+
179+
const createSpec = (methodName: string) => {
180+
const funcSpec = xdr.ScSpecEntry.scSpecEntryFunctionV0(
181+
new xdr.ScSpecFunctionV0({
182+
doc: "",
183+
name: methodName,
184+
inputs: [],
185+
outputs: [xdr.ScSpecTypeDef.scSpecTypeU32()],
186+
}),
187+
);
188+
return new contract.Spec([funcSpec.toXDR("base64")]);
189+
};
190+
191+
function buildInvokeTx(targetContractId: string, methodName: string) {
192+
return new TransactionBuilder(source, {
193+
fee: "100",
194+
networkPassphrase,
195+
})
196+
.setTimeout(TimeoutInfinite)
197+
.addOperation(
198+
Operation.invokeContractFunction({
199+
contract: targetContractId,
200+
function: methodName,
201+
args: [],
202+
}),
203+
)
204+
.build();
205+
}
206+
207+
it("fromXDR() accepts a transaction targeting the configured contract", () => {
208+
const tx = buildInvokeTx(victimContractId, "test");
209+
const xdrBase64 = tx.toEnvelope().toXDR("base64");
210+
const spec = createSpec("test");
211+
212+
const assembled = contract.AssembledTransaction.fromXDR(
213+
{
214+
contractId: victimContractId,
215+
networkPassphrase,
216+
rpcUrl: "https://example.com",
217+
},
218+
xdrBase64,
219+
spec,
220+
);
221+
expect(assembled.built).toBeDefined();
222+
});
223+
224+
it("fromXDR() rejects a transaction targeting a different contract", () => {
225+
const tx = buildInvokeTx(attackerContractId, "drain");
226+
const xdrBase64 = tx.toEnvelope().toXDR("base64");
227+
const spec = createSpec("drain");
228+
229+
expect(() =>
230+
contract.AssembledTransaction.fromXDR(
231+
{
232+
contractId: victimContractId,
233+
networkPassphrase,
234+
rpcUrl: "https://example.com",
235+
},
236+
xdrBase64,
237+
spec,
238+
),
239+
).toThrow(
240+
`Transaction envelope targets contract ${attackerContractId}, but this Client is configured for ${victimContractId}.`,
241+
);
242+
});
243+
244+
it("fromJSON() accepts a transaction targeting the configured contract", () => {
245+
const tx = buildInvokeTx(victimContractId, "test");
246+
const spec = createSpec("test");
247+
const simulationResult = {
248+
auth: [],
249+
retval: xdr.ScVal.scvU32(0).toXDR("base64"),
250+
};
251+
const simulationTransactionData = new SorobanDataBuilder()
252+
.build()
253+
.toXDR("base64");
254+
255+
const json = JSON.stringify({
256+
method: "test",
257+
tx: tx.toEnvelope().toXDR("base64"),
258+
simulationResult,
259+
simulationTransactionData,
260+
});
261+
262+
const { method, ...txData } = JSON.parse(json);
263+
const assembled = contract.AssembledTransaction.fromJSON(
264+
{
265+
contractId: victimContractId,
266+
networkPassphrase,
267+
rpcUrl: "https://example.com",
268+
method,
269+
parseResultXdr: (result: any) => spec.funcResToNative(method, result),
270+
},
271+
txData,
272+
);
273+
expect(assembled.built).toBeDefined();
274+
});
275+
276+
it("fromJSON() rejects a transaction targeting a different contract", () => {
277+
const tx = buildInvokeTx(attackerContractId, "drain");
278+
const simulationResult = {
279+
auth: [],
280+
retval: xdr.ScVal.scvU32(0).toXDR("base64"),
281+
};
282+
const simulationTransactionData = new SorobanDataBuilder()
283+
.build()
284+
.toXDR("base64");
285+
286+
const json = JSON.stringify({
287+
method: "drain",
288+
tx: tx.toEnvelope().toXDR("base64"),
289+
simulationResult,
290+
simulationTransactionData,
291+
});
292+
293+
const { method, ...txData } = JSON.parse(json);
294+
295+
expect(() =>
296+
contract.AssembledTransaction.fromJSON(
297+
{
298+
contractId: victimContractId,
299+
networkPassphrase,
300+
rpcUrl: "https://example.com",
301+
method,
302+
parseResultXdr: () => {},
303+
},
304+
txData,
305+
),
306+
).toThrow(
307+
`Transaction envelope targets contract ${attackerContractId}, but this Client is configured for ${victimContractId}.`,
308+
);
309+
});
310+
311+
it("fromJSON() rejects a transaction with a spoofed method name", () => {
312+
const tx = buildInvokeTx(victimContractId, "transfer");
313+
const simulationResult = {
314+
auth: [],
315+
retval: xdr.ScVal.scvU32(0).toXDR("base64"),
316+
};
317+
const simulationTransactionData = new SorobanDataBuilder()
318+
.build()
319+
.toXDR("base64");
320+
321+
const json = JSON.stringify({
322+
method: "safe_operation",
323+
tx: tx.toEnvelope().toXDR("base64"),
324+
simulationResult,
325+
simulationTransactionData,
326+
});
327+
328+
const { method, ...txData } = JSON.parse(json);
329+
330+
expect(() =>
331+
contract.AssembledTransaction.fromJSON(
332+
{
333+
contractId: victimContractId,
334+
networkPassphrase,
335+
rpcUrl: "https://example.com",
336+
method,
337+
parseResultXdr: () => {},
338+
},
339+
txData,
340+
),
341+
).toThrow(
342+
"Transaction envelope calls method 'transfer', but the provided method is 'safe_operation'.",
343+
);
344+
});
345+
});

0 commit comments

Comments
 (0)