Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
hats-bug-reporter bot opened this issue Nov 13, 2024 · 1 comment
Open

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

hats-bug-reporter bot opened this issue Nov 13, 2024 · 1 comment
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@hats-bug-reporter
Copy link

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.

(bool success, ) = kyberRouter.call{value: msg.value}(targetData);
if (!success) {
revert CallFailed();
}
} else {
amountIn = _resolveTokenValue(tokenIn, amountIn);
IERC20(tokenIn).forceApprove(kyberRouter, amountIn);
(bool success, ) = kyberRouter.call(targetData);
if (!success) {
revert CallFailed();
}

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.

@hats-bug-reporter hats-bug-reporter bot added the bug Something isn't working label Nov 13, 2024
@yanisepfl yanisepfl added the invalid This doesn't seem right label Nov 15, 2024
@yanisepfl
Copy link
Collaborator

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

No branches or pull requests

1 participant