Refactor

Author: quietbits

src/contract/assembled_transaction.ts

@@ -361,53 +361,86 @@ export class AssembledTransaction<T> {
     });
   }
 
-  static fromJSON<T>(
-    options: Omit<AssembledTransactionOptions<T>, "args">,
-    {
-      tx,
-      simulationResult,
-      simulationTransactionData,
-    }: {
-      tx: XDR_BASE64;
-      simulationResult: {
-        auth: XDR_BASE64[];
-        retval: XDR_BASE64;
-      };
-      simulationTransactionData: XDR_BASE64;
-    },
-  ): AssembledTransaction<T> {
-    const txn = new AssembledTransaction(options);
-    txn.built = TransactionBuilder.fromXDR(tx, options.networkPassphrase) as Tx;
-
-    if (txn.built.operations.length !== 1) {
+  /**
+   * Validate that a built transaction is a single invokeContract operation
+   * targeting the expected contract, and return the parsed InvokeContractArgs.
+   */
+  private static validateInvokeContractOp(
+    built: Tx,
+    expectedContractId: string,
+  ): xdr.InvokeContractArgs {
+    if (built.operations.length !== 1) {
       throw new Error(
         "Transaction envelope must contain exactly one operation.",
       );
     }
 
-    const operation = txn.built.operations[0] as Operation.InvokeHostFunction;
+    const operation = built.operations[0] as Operation.InvokeHostFunction;
+
+    if (!operation?.func?.switch) {
+      throw new Error(
+        "Transaction envelope does not contain an invokeHostFunction operation.",
+      );
+    }
 
-    if (!operation?.func?.value || typeof operation.func.value !== "function") {
+    if (operation.func.switch().name !== "hostFunctionTypeInvokeContract") {
       throw new Error(
-        "Could not extract the method from the transaction envelope.",
+        "Transaction envelope does not contain an invokeContract host function.",
       );
     }
 
     const invokeContractArgs = operation.func.value() as xdr.InvokeContractArgs;
 
-    if (!invokeContractArgs?.functionName) {
+    if (
+      !invokeContractArgs?.contractAddress ||
+      !invokeContractArgs?.functionName
+    ) {
       throw new Error(
-        "Could not extract the method name from the transaction envelope.",
+        "Could not extract contract address or method name from the transaction envelope.",
       );
     }
 
     const xdrContractId = Address.fromScAddress(
       invokeContractArgs.contractAddress(),
     ).toString();
 
-    if (xdrContractId !== options.contractId) {
+    if (xdrContractId !== expectedContractId) {
+      throw new Error(
+        `Transaction envelope targets contract ${xdrContractId}, but this Client is configured for ${expectedContractId}.`,
+      );
+    }
+
+    return invokeContractArgs;
+  }
+
+  static fromJSON<T>(
+    options: Omit<AssembledTransactionOptions<T>, "args">,
+    {
+      tx,
+      simulationResult,
+      simulationTransactionData,
+    }: {
+      tx: XDR_BASE64;
+      simulationResult: {
+        auth: XDR_BASE64[];
+        retval: XDR_BASE64;
+      };
+      simulationTransactionData: XDR_BASE64;
+    },
+  ): AssembledTransaction<T> {
+    const txn = new AssembledTransaction(options);
+    txn.built = TransactionBuilder.fromXDR(tx, options.networkPassphrase) as Tx;
+
+    const invokeContractArgs = AssembledTransaction.validateInvokeContractOp(
+      txn.built,
+      options.contractId,
+    );
+
+    const xdrMethod = invokeContractArgs.functionName().toString("utf-8");
+
+    if (xdrMethod !== options.method) {
       throw new Error(
-        `Transaction envelope targets contract ${xdrContractId}, but this Client is configured for ${options.contractId}.`,
+        `Transaction envelope calls method '${xdrMethod}', but the provided method is '${options.method}'.`,
       );
     }
 
@@ -453,34 +486,10 @@ export class AssembledTransaction<T> {
       options.networkPassphrase,
     ) as Tx;
 
-    if (built.operations.length !== 1) {
-      throw new Error(
-        "Transaction envelope must contain exactly one operation.",
-      );
-    }
-
-    const operation = built.operations[0] as Operation.InvokeHostFunction;
-    if (!operation?.func?.value || typeof operation.func.value !== "function") {
-      throw new Error(
-        "Could not extract the method from the transaction envelope.",
-      );
-    }
-    const invokeContractArgs = operation.func.value() as xdr.InvokeContractArgs;
-    if (!invokeContractArgs?.functionName) {
-      throw new Error(
-        "Could not extract the method name from the transaction envelope.",
-      );
-    }
-
-    const xdrContractId = Address.fromScAddress(
-      invokeContractArgs.contractAddress(),
-    ).toString();
-
-    if (xdrContractId !== options.contractId) {
-      throw new Error(
-        `Transaction envelope targets contract ${xdrContractId}, but this Client is configured for ${options.contractId}.`,
-      );
-    }
+    const invokeContractArgs = AssembledTransaction.validateInvokeContractOp(
+      built,
+      options.contractId,
+    );
 
     const method = invokeContractArgs.functionName().toString("utf-8");
     const txn = new AssembledTransaction({

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

@@ -307,4 +307,39 @@ describe("Contract ID validation on deserialization", () => {
       `Transaction envelope targets contract ${attackerContractId}, but this Client is configured for ${victimContractId}.`,
     );
   });
+
+  it("fromJSON() rejects a transaction with a spoofed method name", () => {
+    const tx = buildInvokeTx(victimContractId, "transfer");
+    const simulationResult = {
+      auth: [],
+      retval: xdr.ScVal.scvU32(0).toXDR("base64"),
+    };
+    const simulationTransactionData = new SorobanDataBuilder()
+      .build()
+      .toXDR("base64");
+
+    const json = JSON.stringify({
+      method: "safe_operation",
+      tx: tx.toEnvelope().toXDR("base64"),
+      simulationResult,
+      simulationTransactionData,
+    });
+
+    const { method, ...txData } = JSON.parse(json);
+
+    expect(() =>
+      contract.AssembledTransaction.fromJSON(
+        {
+          contractId: victimContractId,
+          networkPassphrase,
+          rpcUrl: "https://example.com",
+          method,
+          parseResultXdr: () => {},
+        },
+        txData,
+      ),
+    ).toThrow(
+      "Transaction envelope calls method 'transfer', but the provided method is 'safe_operation'.",
+    );
+  });
 });