Steadefi
Steadefi
DeFiHardhatFoundryOracle
35,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

No check if variable `UpperLimit` > variable `LowerLimit`

SBSecurity

Summary

There are no checks if state variables LowerLimit is lower than the UpperLimit.

Vulnerability Details

This arises for two variables _store.delta and debtRatio. In the GMXVault constructor _store.debtRatioUpperLimit and _store.debtRatioLowerLimit but we do not check if LowerLimit < UpperLimit, the same applies for _store.deltaUpperLimit and _store.deltaLowerLimit.

The issue arises in the GMXVault constructor and in the updateParameterLimits().

Impact

If somehow these parameters are messed up, then will break the protocol rebalance logic.

Tools Used

Manual

Recommendations

constructor (
string memory name,
string memory symbol,
GMXTypes.Store memory store_
) ERC20(name, symbol) Ownable(msg.sender) {
_store.leverage = uint256(store_.leverage);
_store.delta = store_.delta;
_store.feePerSecond = uint256(store_.feePerSecond);
_store.treasury = address(store_.treasury);
​
_store.debtRatioStepThreshold = uint256(store_.debtRatioStepThreshold);
+ if (store_.debtRatioUpperLimit < store_.debtRatioLowerLimit) {
+ revert Errors.DebtRatioLimitsNotReal();
+ }
​
_store.debtRatioUpperLimit = uint256(store_.debtRatioUpperLimit);
_store.debtRatioLowerLimit = uint256(store_.debtRatioLowerLimit);
​
+ if (store_.deltaUpperLimit < store_.deltaLowerLimit) {
+ revert Errors.DeltaLimitsNotReal();
+ }
​
_store.deltaUpperLimit = int256(store_.deltaUpperLimit);
_store.deltaLowerLimit = int256(store_.deltaLowerLimit);
_store.minSlippage = store_.minSlippage;
_store.minExecutionFee = store_.minExecutionFee;
function updateParameterLimits(
uint256 debtRatioStepThreshold,
uint256 debtRatioUpperLimit,
uint256 debtRatioLowerLimit,
int256 deltaUpperLimit,
int256 deltaLowerLimit
) external onlyOwner {
_store.debtRatioStepThreshold = debtRatioStepThreshold;
​
+ if (debtRatioUpperLimit < debtRatioLowerLimit) {
+ revert Errors.DebtRatioLimitsNotReal();
+ }
​
_store.debtRatioUpperLimit = debtRatioUpperLimit;
_store.debtRatioLowerLimit = debtRatioLowerLimit;
​
+ if (deltaUpperLimit < deltaLowerLimit) {
+ revert Errors.DeltaLimitsNotReal();
+ }
​
_store.deltaUpperLimit = deltaUpperLimit;
_store.deltaLowerLimit = deltaLowerLimit;
​
emit ParameterLimitsUpdated(
debtRatioStepThreshold,
debtRatioUpperLimit,
debtRatioLowerLimit,
deltaUpperLimit,
deltaLowerLimit
);
}
Updates

Lead Judging Commences

hans Lead Judge almost 2 years ago
Submission Judgement Published
Validated
Assigned finding tags:

Centralization Risk

Impact: High Likelihood: Low Centralization risk is regarded a known issue. This tag will include all submissions : - Admin setter functions without validations

slavcheww Submitter
almost 2 years ago
hans Auditor
almost 2 years ago
hans Auditor
almost 2 years ago
hans Lead Judge almost 2 years ago
Submission Judgement Published
Invalidated
Reason: Known issue
Assigned finding tags:

Centralization Risk

Impact: High Likelihood: Low Centralization risk is regarded a known issue. This tag will include all submissions : - Admin setter functions without validations

Support

FAQs

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