Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

Improper Fee Basis Points Validation Leading to Excessive Total Basis Points

juggernaut63

Summary

The basis points of all fees can exceed 10,000 (100%). This can happen because in the addFee and updateFee functions, although there is validation so that the total basis points of all fees do not exceed 10,000, this validation is carried out after the addition or refresh of the fee is carried out.

Vulnerability Details

function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner {
@=> fees.push(Fee(_receiver, _feeBasisPoints));
@=> if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}
​
function updateFee(
uint256 _index,
address _receiver,
uint256 _feeBasisPoints
) external onlyOwner {
require(_index < fees.length, "Fee does not exist");
​
@=> if (_feeBasisPoints == 0) {
fees[_index] = fees[fees.length - 1];
fees.pop();
} else {
fees[_index].receiver = _receiver;
fees[_index].basisPoints = _feeBasisPoints;
}
​
@=> if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}

The issue arises because the validation of total basis points occurs after the addition or update of a fee. This allows the total basis points to temporarily exceed the maximum allowed value of 10,000.

Scenario:

If the total basis points of all fees are already 9,000 and then you add a new fee of 2,000 basis points, then the total basis points will be 11,000 which exceeds the limit of 10,000. However, the validation to check the total basis points is done after the new fee is added, so the addition will still be accepted.

The same case also occurs in:

StakingPool

VaultControllerStrategy

Impact

In cases where the total basis points already exceed 10,000 before the fee is added or updated, such validation will not prevent the addition or update of the fee.

Tools Used

Manual review

Recommendations

validate the total basis points before modifying the fee structure.

function addFee(address _receiver, uint256 _feeBasisPoints) external onlyOwner {
- fees.push(Fee(_receiver, _feeBasisPoints));
- if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
+ uint256 newTotal = _totalFeesBasisPoints() + _feeBasisPoints;
+ if (newTotal > 10000) revert FeesExceedLimit();
+ fees.push(Fee(_receiver, _feeBasisPoints));
}
​
function updateFee(
uint256 _index,
address _receiver,
uint256 _feeBasisPoints
) external onlyOwner {
require(_index < fees.length, "Fee does not exist");
​
+ uint256 currentFeeBasisPoints = fees[_index].basisPoints;
+ uint256 newTotal = _totalFeesBasisPoints() - currentFeeBasisPoints + _feeBasisPoints;
+ if (newTotal > 10000) revert FeesExceedLimit();
​
if (_feeBasisPoints == 0) {
fees[_index] = fees[fees.length - 1];
fees.pop();
} else {
fees[_index].receiver = _receiver;
fees[_index].basisPoints = _feeBasisPoints;
}
​
- if (_totalFeesBasisPoints() > 10000) revert FeesExceedLimit();
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 8 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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