Hi, I noticed a possible binding issue between the selected agent and the settlement recipient.
In src/core.cairo, the workflow entrypoint accepts both payee_address and target_agent_id as separate parameters:
5: fn execute_agent_workflow(
6: ref self: TContractState,
7: payer_address: ContractAddress,
8: payee_address: ContractAddress,
9: token_address: ContractAddress,
10: amount: u256,
11: task_id: felt252,
12: deadline: u64,
13: signature: Array<felt252>,
14: target_agent_id: u256,
15: score: u8,
16: );
Later, the core checks that the target agent is active, but it passes the independent payee_address to settlement and records feedback for target_agent_id:
92: let target_agent = identity.get_agent(target_agent_id);
93: assert(target_agent.is_active, 'TARGET_AGENT_INACTIVE');
94:
95: settlement
96: .settle_payment(
97: payer_address,
98: payee_address,
99: token_address,
100: amount,
101: task_id,
102: deadline,
103: signature,
104: );
105: reputation.submit_feedback(target_agent_id, task_id, score);
In src/settlement.cairo, the settlement hash does bind the payer, payee, token, amount, task, and deadline:
16: PoseidonTrait::new()
17: .update('TALOS_SETTLEMENT')
18: .update('v1')
19: .update(verifying_contract.into())
20: .update(chain_id)
21: .update(payer_address.into())
22: .update(payee_address.into())
23: .update(token_address.into())
24: .update(amount.low.into())
25: .update(amount.high.into())
26: .update(task_id)
27: .update(deadline.into())
28: .finalize()
And settlement transfers to that payee after signature verification:
196: let message_hash = compute_mock_message_hash(
197: payer_address, payee_address, token_address, amount, task_id, deadline,
198: );
199: let payer_account = ISRC6Dispatcher { contract_address: payer_address };
200: let is_valid_signature_felt = payer_account.is_valid_signature(message_hash, signature);
203: assert(is_valid_signature, 'INVALID_SIGNATURE');
...
208: let token = IERC20Dispatcher { contract_address: token_address };
209: let transferred = token.transfer_from(payer_address, payee_address, amount);
210: assert(transferred, 'TRANSFER_FAILED');
The concern is not that settlement is unsigned. The concern is that the core workflow appears to allow this split:
target_agent_id -> active check and feedback
payee_address -> settlement recipient
I did not see an invariant such as payee_address == target_agent.owner or payee_address == target_agent.payment_address before settlement.
A safer design would enforce an on-chain binding between the target agent and the recipient address before calling settle_payment. A regression test with an active agent and a different valid payee would make the intended invariant clear.
I am reporting this as a potential issue rather than a confirmed exploit, since a trusted UI or relayer may currently pass matching values, but the contract entrypoint itself appears to rely on that off-chain discipline.
Hi, I noticed a possible binding issue between the selected agent and the settlement recipient.
In
src/core.cairo, the workflow entrypoint accepts bothpayee_addressandtarget_agent_idas separate parameters:Later, the core checks that the target agent is active, but it passes the independent
payee_addressto settlement and records feedback fortarget_agent_id:In
src/settlement.cairo, the settlement hash does bind the payer, payee, token, amount, task, and deadline:And settlement transfers to that payee after signature verification:
The concern is not that settlement is unsigned. The concern is that the core workflow appears to allow this split:
I did not see an invariant such as
payee_address == target_agent.ownerorpayee_address == target_agent.payment_addressbefore settlement.A safer design would enforce an on-chain binding between the target agent and the recipient address before calling
settle_payment. A regression test with an active agent and a different valid payee would make the intended invariant clear.I am reporting this as a potential issue rather than a confirmed exploit, since a trusted UI or relayer may currently pass matching values, but the contract entrypoint itself appears to rely on that off-chain discipline.