Lack of Target Data Validation in KyberSwap Command Execution

[hats-bug-reporter]

Github username: @0xmahdirostami
Twitter username: 0xmahdirostami
Submission hash (on-chain): 0x4a7a612fd62ef9364c23b61b1f39f0277f2ed86f38f288f403cc2da0f79386c3
Severity: high

Description:

Description

In the current implementation of the KyberSwap command, the contract validates only the user's input data but fails to verify targetData passed to the KyberRouter. This targetData is generated off-chain, allowing the msg.sender to manipulate it without the contract enforcing consistency between the user input and the actual swap parameters.

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Dispatcher.sol

Lines 330 to 338 in 06b05fc

	(address tokenIn, uint256 amountIn, address tokenOut, , bytes memory targetData) = abi
	.decode(_inputs, (address, uint256, address, uint256, bytes));
	if (tokenOut == Constants.ETH) {
	revert AddressError();
	}
	if (tokenIn == Constants.ETH) {
	if (msg.value != amountIn) {
	revert AmountError();
	}

however, contract calls the kyberRouter with target data.

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Dispatcher.sol

Line 339 in 06b05fc

(bool success, ) = kyberRouter.call{value: msg.value}(targetData);

Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/src/router/Dispatcher.sol

Line 346 in 06b05fc

(bool success, ) = kyberRouter.call(targetData);

Since there is no validation on targetData, malicious users could craft off-chain data that differs from the input data, allowing potential exploits during command execution.

Impact

• Vulnerabilities in Off-Chain Data Generation:

If the off-chain generator of targetData is compromised, there are no safeguards within the contract to protect users from incorrect trade parameters or invalid routes.
consider the fact that _dispatchPreviewRate will show wrong value here as it works with user input not actual targetData

---

• Unauthorized Contract Balance Use:

If the contract holds a balance, an attacker could submit msg.value and input amount as minimal values (e.g., 1 wei) but encode targetData with the full contract balance, allowing them to drain contract funds.

---

• Partial Fill Exploit:

If partial fills are enabled, it could lead to unintended losses for users. To avoid such scenarios, other protocols ensure that partial fill is disabled. For reference:

i wrote this contract, this is about a 1inch swap but it could give some sense of what i say:
https://github.com/PossumLabsCrypto/Adapters/blob/ac3effb65bf1b72dcadde1059b26345b5b969284/src/AdapterV1.sol#L516

---

• Misalignment of Input and Execution Data:

The contract currently assumes that the user-provided inputs align with target data. This assumption could lead to trade execution errors or incorrect balances if the values differ.

Mitigation

To mitigate this risk, the contract should decode targetData internally and validate it against user inputs. This ensures that swap parameters match the expected input, preventing any inconsistencies or exploitative scenarios.

[yanisepfl]

Hello,

We classified this issue as Invalid because we consider this to be Kyber router's responsibility to revert upon reception of malicious data.

You will find more information on how to interact with their router here: https://docs.kyberswap.com/kyberswap-solutions/kyberswap-aggregator/developer-guides/execute-a-swap-with-the-aggregator-api.

Thanks