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

Lows - 4

pontifex

L-1

Summary

The protocol is not compliant with wired tokens. Particularly fee-on-transfer tokens.

Vulnerability Details

The protocol uses user's input as deposited amount:

IERC20(dp.token).safeTransferFrom(msg.sender, address(this), dp.amt);

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXDeposit.sol#L83

Impact

Assets loses

Tools used

Manual Review

Recommendations

Compute the balance before and after transfer and subtract them to get the real amount. Also use nonReentrant while using this to prevent from reentrancy in ERC777 tokens.

L-2

Summary

The protocol is not compliant with tokens which have decimals more than 18.

Vulnerability Details

In the lines below the protocol will always revert for tokens with decimals more than 18.
https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXReader.sol#L67
https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXManager.sol#L198
https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXManager.sol#L208

Impact

Underflow reverts

Tools used

Manual Review

Recommendations

Use division instead of subtracting for decimals normalizing.

L-3

Summary

Useless check

Vulnerability Details

The checks at lines L#252 and L#265 do nothing because uints can’t be less than zero.

249 function addTokenMaxDelay(address token, uint256 maxDelay) external onlyOwner {
​
252 if (maxDelay < 0) revert Errors.TokenPriceFeedMaxDelayMustBeGreaterOrEqualToZero();
​
262 function addTokenMaxDeviation(address token, uint256 maxDeviation) external onlyOwner {
​
265 if (maxDeviation < 0) revert Errors.TokenPriceFeedMaxDeviationMustBeGreaterOrEqualToZero();

Impact

No impact

Tools used

Manual Review

Recommendations

Remove this checks.

L-4

Summary

Underflow in unchecked brackets

Vulnerability Details

There can be underflow in the GMXEmergency.emergencyWithdraw function. This can cause unexpected reveret at the _burn.

unchecked {
if (_userShareBalance - shareAmt < DUST_AMOUNT) {
shareAmt = _userShareBalance;
}
}

https://github.com/Cyfrin/2023-10-SteadeFi/blob/0f909e2f0917cb9ad02986f631d622376510abec/contracts/strategy/gmx/GMXEmergency.sol#L170-L174

Impact

Unexpected insufficient balance error

Tools used

Manual Review

Recommendations

I suppose there is no need in gas economy:

- unchecked {
- if (_userShareBalance - shareAmt < DUST_AMOUNT) {
+ if (_userShareBalance != shareAmt && _userShareBalance - shareAmt < DUST_AMOUNT) {
shareAmt = _userShareBalance;
}
- }
Updates

Lead Judging Commences

hans Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity
Assigned finding tags:

INFO: Unnecessary maxDelay/maxDeviation check

Redundant check on maxDelay and/or maxDeviation in ARBOracle

Support

FAQs

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