code-423n4 / 2023-05-maia-findings

20 stars 12 forks source link

BranchPort.toggleStrategyToken used on unregistered STRATEGY TOKEN will allow STRATEGIES to drain full token balance #915

Closed code423n4 closed 1 year ago

code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-omnichain/BranchPort.sol#L342-L346 https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-omnichain/BranchPort.sol#L331-L339 https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-omnichain/BranchPort.sol#L149-L151

Vulnerability details

Impact

BranchPort.toggleStrategyToken may be called on a token not registered as a strategy token effectively registering it without setting a getMinimumTokenReserveRatio. In such a case _minimumReserves will always return a value smaller than the current balance which in turn will make _minimumReserves always return some reserves available while _reservesLacking always return 0.

function _minimumReserves(uint256 _currBalance, address _token) internal view returns (uint256) {
        return ((_currBalance + getStrategyTokenDebt[_token]) * getMinimumTokenReserveRatio[_token]) / DIVISIONER;
    }

Tools Used

Pen and paper

Recommended Mitigation Steps

Add check to BranchPort.toggleStrategyToken

Assessed type

Other

c4-judge commented 1 year ago

trust1995 marked the issue as unsatisfactory: Insufficient proof