yanisepfl
commented
1 week ago 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
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.
https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/router/Dispatcher.sol#L330-L338
however, contract calls the kyberRouter with target data.
https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/router/Dispatcher.sol#L339
https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/router/Dispatcher.sol#L346
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
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
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.
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
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.