hats-finance / Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d

Other
0 stars 1 forks source link

Lack of Contract Existence Check on Low-Level KyberSwap Call #15

Open hats-bug-reporter[bot] opened 1 week ago

hats-bug-reporter[bot] commented 1 week ago

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

Description:

Description

The router contract currently uses a low-level call for executing swaps on KyberSwap, without verifying that the target address is a valid, existing contract. Since the EVM design allows low-level call to return “success” even when the target address is non-existent or points to a destroyed contract, this can lead to unintended behavior. If the Kyber router address is set incorrectly or if the router contract at that address is destroyed, calls will still return success, leading users to mistakenly believe their swap was executed when it was not.

https://github.com/hats-finance/Spectra-0x4b792db3d2a5d1c1ccf9938380756b200c240e5d/blob/06b05fc261fe59b1fe25e308a10a16459289a4be/src/router/Dispatcher.sol#L339-L349

Scenario

  1. The owner updates the Kyber router address to a new official address.
  2. The new Kyber router address points to a contract with a selfdestruct function.
  3. At some point, the new Kyber router contract self-destructs or is otherwise destroyed.
  4. A user initiates a swap using the router, but due to the missing contract existence check, the call succeeds without actually performing the swap, resulting in potential fund loss.

another scenario will be the owner setting a Kyber router address to the wrong address.

Impact

This oversight could lead to users losing funds, as they may not receive their expected swap returns if the Kyber router contract is unavailable or destroyed.

Mitigation

To prevent this issue:

  1. Contract Existence Check: Before executing a low-level call, implement a check to ensure the target address is a deployed contract. Solidity has various methods for this check.

  2. Balance Verification: Before and after each Kyber router call, verify that the minAmountOut criteria have been met by comparing token balances.

yanisepfl commented 1 week ago

Hello,

We classified this issue as Invalid because we assume the owner is not malicious, and is responsible for checking that the correct Kyber Router address is being used, and we assume that Kyber will not selfdestruct their router without any warning.

Thanks