RebateFi Hook
First Flight #53
Beginner FriendlyDeFi
100 EXP
View results
Previous Next
Submission Details
Impact: low
Likelihood: low
Invalid

Missing check for outcome of the transaction - If the transaction fails, the function will continue normally.

farnad

Root + Impact

Description

  • In the ReFiSwapRebateHook::withdrawTokens function, if for any reason the transaction fails, the code does not catch the issue and revert (while it is supposed to). Instead, it continues and emits the event. It may cause confusion for external services or parties relying on the event.


  • Even if the returning result is accounted for, in some cases of weird ERC20 (like for USDT which does not return any value), it will be impossible to notice the transaction failure.

function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
@> IERC20(token).transfer(to, amount);
emit TokensWithdrawn(to, token, amount);
}

Risk

Likelihood: Low

  • It will happen when something goes wrong during execution of the transaction in the blockchain.


Impact: Low

  • Since the event is emitted indefinitely even after the unsuccessful transaction, the owner may wrongly assume it was done properly and move on.

Proof of Concept

You can see the emitted event by:

  1. Copying/pasting the code into Remix.

  2. Running the program.

  3. Seeing the event in the terminal window.

​

Recommended Mitigation

The easy solution is to get the return value of the transfer function and check if it is true. If not, revert with the appropriate custom error. However, it is not effective in the case of weird ERC20 tokens like USDT which does not return a value.

The best solution is to utilize the safeTransfer function from Openzeppelin SafeERC20 library.

+import {SafeERC20} from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
contract ReFiSwapRebateHook is BaseHook, Ownable {
+ using SafeERC20 for IERC20;
.
.
.
function withdrawTokens(address token, address to, uint256 amount) external onlyOwner {
- IERC20(token).transfer(to, amount);
+ IERC20(token).safeTransfer(to, amount);
emit TokensWithdrawn(to, token, amount);
}
Updates

Lead Judging Commences

chaossr Lead Judge 11 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Appeal created

farnad Submitter
9 days ago
chaossr Lead Judge
9 days ago
chaossr Lead Judge 6 days ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!