Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

If a STRATEGY TOKEN is "Toggled off" STRATEGIES will still be able to withdraw but returning of tokens with replenishReserves will be disabled. #882

Open
code423n4 opened this issue Jul 5, 2023 · 6 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working H-01 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")

Comments

@code423n4
Copy link
Contributor

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 to manage() while replenishTokens will not be callable on them and thus tokens won't be force returnable.

Tools Used

Pen and Paper.

Recommended Mitigation Steps

  1. Add check on enabled strategy token in manage()
  2. Validate getPortStrategyTokenDebt[_strategy][_token] > 0 instead of !isStrategyToken[_token] in replenishReserves()

Assessed type

Access Control

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Jul 5, 2023
code423n4 added a commit that referenced this issue Jul 5, 2023
@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Jul 11, 2023
@c4-judge
Copy link
Contributor

trust1995 marked the issue as unsatisfactory:
Insufficient proof

@c4-sponsor c4-sponsor added the sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") label Jul 12, 2023
@c4-sponsor
Copy link

0xBugsy marked the issue as sponsor confirmed

@c4-judge c4-judge reopened this Jul 25, 2023
@c4-judge c4-judge added satisfactory satisfies C4 submission criteria; eligible for awards and removed unsatisfactory does not satisfy C4 submission criteria; not eligible for awards labels Jul 25, 2023
@c4-judge
Copy link
Contributor

trust1995 marked the issue as satisfactory

@c4-judge
Copy link
Contributor

trust1995 marked the issue as primary issue

@c4-judge c4-judge added primary issue Highest quality submission among a set of duplicates selected for report This submission will be included/highlighted in the audit report labels Jul 25, 2023
@c4-judge
Copy link
Contributor

trust1995 marked the issue as selected for report

@0xLightt
Copy link

0xLightt commented Sep 6, 2023

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working H-01 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity")
Projects
None yet
Development

No branches or pull requests

5 participants