Open code423n4 opened 1 year ago
trust1995 marked the issue as unsatisfactory: Insufficient proof
0xBugsy marked the issue as sponsor confirmed
trust1995 marked the issue as satisfactory
trust1995 marked the issue as primary issue
trust1995 marked the issue as selected for report
Lines of code
https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-omnichain/BranchPort.sol#L158-L169 https://github.com/code-423n4/2023-05-maia/blob/54a45beb1428d85999da3f721f923cbf36ee3d35/src/ulysses-omnichain/BranchPort.sol#L172-L186
Vulnerability details
Impact
BranchPort.manage
allows a registered Strategy to withdraw certain amounts of enabled strategy tokens. It validates access rights ie. if called by a strategy registered for the requested token. It however doesn't check if the token itself is currently enabled.Conversely
BranchPort.replenishTokens
allows to force withdraw managed tokens from a strategy. It however performs a check if the token is currently an active strategy token.Strategy token may be disabled by
toggleStrategyToken()
even if there are active strategies managing it active. In such case these strategies will still be able to withdraw the tokens with calls tomanage()
whilereplenishTokens
will not be callable on them and thus tokens won't be force returnable.Tools Used
Pen and Paper.
Recommended Mitigation Steps
manage()
getPortStrategyTokenDebt[_strategy][_token] > 0
instead of!isStrategyToken[_token]
inreplenishReserves()
Assessed type
Access Control