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

Due to the missing validation for fee limits, they can be set to too high values and break the functionality of the protocol.

farnad

Root + Impact

Description

  • When changing the fees in the ReFiSwapRebateHook::ChangeFee function, the new fee arguments are not checked to have proper (not too high) values.

  • They need to be checked against MAX_LP_FEE. In simple words, it means the fees must be lower than 100% for the dapp to work properly.

  • Otherwise, all the users' tokens will be spent just to cover the fees. And they will get nothing in return.

function ChangeFee(bool _isBuyFee, uint24 _buyFee, bool _isSellFee, uint24 _sellFee) external onlyOwner {
@> // The fees arguments validation should happen here. Currently, it is missing.
if (_isBuyFee) buyFee = _buyFee;
if (_isSellFee) sellFee = _sellFee;
}
​

Risk

Likelihood: Medium

  • It is fairly possible for the owner to mistakenly set high fee percentages for the transactions.

Impact: High

  • In absence of the proper checking, it will go through and severely affect the functionality of the protocol.

  • Users will NOT get any tokens in return when they try to swap their own tokens.

Proof of Concept

Please copy/paste the following function into the test file and run it.

function test_ownerCanSetTooHighFees() public {
// Setting too high fees does not revert in the current implementation.
// Arrange
uint24 newFee = 1_100_000; // 110%
// Act
vm.startPrank(rebateHook.owner());
rebateHook.ChangeFee(true, newFee, true, newFee);
vm.stopPrank();
​
// Assert
(uint24 buyFee, uint24 sellFee) = rebateHook.getFeeConfig();
assertEq(sellFee, newFee, "Cannot set too high sell fee in current implementation");
​
assertEq(buyFee, newFee, "Cannot set too high buy fee in current implementation");
}

Recommended Mitigation

In order to solve the issue, please make the following changes.

+ error FeeTooHigh(uint24 attemptedFee, uint24 maxAllowedFee);
.
.
.
function ChangeFee(bool _isBuyFee, uint24 _buyFee, bool _isSellFee, uint24 _sellFee) external onlyOwner {
+ if (_buyFee >= LPFeeLibrary.MAX_LP_FEE)
+ revert FeeTooHigh(_buyFee, LPFeeLibrary.MAX_LP_FEE);
+ if (_sellFee >= LPFeeLibrary.MAX_LP_FEE)
+ revert FeeTooHigh(_sellFee, LPFeeLibrary.MAX_LP_FEE);
​
if (_isBuyFee) buyFee = _buyFee;
if (_isSellFee) sellFee = _sellFee;
}
Updates

Lead Judging Commences

chaossr Lead Judge 12 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!