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

Bent Concrete Crocodile - {actor} will {impact} {affected party} #250

Open
sherlock-admin2 opened this issue Dec 31, 2024 · 0 comments
Open

Comments

@sherlock-admin2
Copy link
Contributor

Bent Concrete Crocodile

High

{actor} will {impact} {affected party}

Summary

https://github.com/sherlock-audit/2024-12-numa-audit/blob/main/Numa/contracts/NumaProtocol/VaultManager.sol#L401

From the comment, the intended logic is that the higher of the two sell fees—either the regular sell fee (lastSellFee) or the sell fee calculated under the critical CF (Collateral Factor) (sell_fee_criticalCF)—should be used as the final fee rate.

The rationale is that when the system is in a state of “extreme danger” or “near insolvency,” the sell fee should increase to discourage further selling, deter malicious behavior, or subsidize system safety.

However, the actual implementation does the opposite: if sell_fee_criticalCF is less than sell_fee_result, the final fee is set to the lower sell_fee_criticalCF. This completely contradicts the intention stated in the comment, as it effectively selects the lower fee instead of the higher one.

Impacts of the Issue

  1. Conflict Between Functionality and Comments
    • The comment specifies, “Whichever sell fee is greater should be used…”, but the code selects the smaller fee instead.
    • This significantly disrupts the intended logic for sell fees: in the most dangerous scenarios, the sell fee is reduced rather than increased, undermining the safety design.

  2. Potential for Economic Risk
    • Under cf_critical conditions (a highly dangerous state), the calculated sell_fee_criticalCF is supposed to be higher to protect the system.
    • However, the current implementation applies a min() operation, potentially lowering the sell fee to a level that fails to deter heavy sell pressure or to collect sufficient fees to subsidize system deficits.

  3. Exploitation by Attackers
    • When the system is in a state of under-collateralization or extreme danger, the intention is to discourage selling and stabilize the market by imposing additional sell fees.
    • This bug, however, allows the fee to drop to the lower value at the most critical moment, giving attackers a window of opportunity to exploit the system.

Root Cause

No response

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

No response

PoC

No response

Mitigation

Based on the comment “Whichever sell fee is greater should be used”, the fix is straightforward: modify the condition to select the higher fee of the two

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant