Issue 327 from previous contest has not been fixed

Summary

Issue 327 has not been fixed. The issue is labeled as "Won't Fix" but dev comments indicates that they are still meant to be fixed. Comments from discord:

Watson Question:

@Gornutz | Blueberry I assume findings that address issues that were marked as "won't fix" in the previous contest are not valid, is that correct?

Dev Response:

they were fixed but not by the solution provided

Vulnerability Detail

BasicSpell.sol#L198-L207

function _validateMaxPosSize(uint256 strategyId) internal view {
    Strategy memory strategy = strategies[strategyId];
    IERC20Upgradeable lpToken = IERC20Upgradeable(strategy.vault);
    uint256 lpBalance = lpToken.balanceOf(address(this));
    uint256 lpPrice = bank.oracle().getPrice(address(lpToken));
    uint256 curPosSize = (lpPrice * lpBalance) /
        10 ** IERC20MetadataUpgradeable(address(lpToken)).decimals();
    if (curPosSize > strategy.maxPositionSize)
        revert Errors.EXCEED_MAX_POS_SIZE(strategyId);
}

We see above that _validateMaxPosSize still uses the lpToken.balnceOf which as pointed out by Issue 327 from the previous contest does not actually prevent users from exceeding the max position size.

Impact

Users can still bypass position size limit

Code Snippet

BasicSpell.sol#L198-L207

Tool used

Manual Review

Recommendation

See Issue 327

Ch-301 commented 1 year ago

Escalate for 10 USDC

It's not logical to keep reporting "Won't Fix" issues in an updated contest e.g.on GMX we have about 20 issues with the lap "Won't Fix" if this is a valid issue and that's okay, so If anyone re-reports all the 20 issues on GMX update contest it will be valid also!!! I think we have the same rules in all the contests.

sherlock-admin commented 1 year ago

Escalate for 10 USDC

It's not logical to keep reporting "Won't Fix" issues in an updated contest e.g.on GMX we have about 20 issues with the lap "Won't Fix" if this is a valid issue and that's okay, so If anyone re-reports all the 20 issues on GMX update contest it will be valid also!!! I think we have the same rules in all the contests.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

ctf-sec commented 1 year ago

Agree with Ch-301 comment

hrishibhat commented 1 year ago

Escalation accepted

Invalid issue Sherlock does not consider issues marked as Wont fix from previous contests as valid submissions in future contests unless mentioned otherwise in the readme.