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

Upgraded Q -> 2 from #167 [1728301859344] #175

Closed
c4-judge opened this issue Oct 7, 2024 · 2 comments
Closed

Upgraded Q -> 2 from #167 [1728301859344] #175

c4-judge opened this issue Oct 7, 2024 · 2 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value duplicate-31 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)

Comments

@c4-judge
Copy link
Contributor

c4-judge commented Oct 7, 2024

Judge has assessed an item in Issue #167 as 2 risk. The relevant finding follows:

[LOW-5] Removal of liquidity also requires the pool to be enabled:

In LOC-687, lib.rs, it is said that liquidity removal from a position does not require the pool to be enabled:

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/lib.rs#L687

    /// Requires token approvals to be set if adding liquidity. Requires the caller to be the
    /// position owner. Requires the pool to be enabled unless removing liquidity.

However, the function update_position() in pool.rs, which is called by update_position_internal() in lib.rs requires the pool to be enabled:

https://github.com/code-423n4/2024-08-superposition/blob/main/pkg/seawater/src/pool.rs#L92

assert_or!(self.enabled.get(), Error::PoolDisabled);

Recommended Mitigation Steps:

Consider allowing removal of liquidity even when the pool is not enabled, that is, when delta is negative

@c4-judge c4-judge added the 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value label Oct 7, 2024
@c4-judge c4-judge closed this as completed Oct 7, 2024
@c4-judge
Copy link
Contributor Author

c4-judge commented Oct 7, 2024

alex-ppg marked the issue as duplicate of #31

@c4-judge c4-judge added the partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%) label Oct 7, 2024
@c4-judge
Copy link
Contributor Author

c4-judge commented Oct 7, 2024

alex-ppg marked the issue as partial-50

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value duplicate-31 partial-50 Incomplete articulation of vulnerability; eligible for partial credit only (50%)
Projects
None yet
Development

No branches or pull requests

1 participant