sherlock-audit / 2023-01-ajna-judging

1 stars 0 forks source link

yixxas - Lenders are forced to pay early withdrawal penalty when they remove quoteToken if pool has no collateral yet #88

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

yixxas

high

Lenders are forced to pay early withdrawal penalty when they remove quoteToken if pool has no collateral yet

Summary

Lenders can add/remove quote token from the pool. They are required to pay an early withdrawal penalty if quote token is removed from above the PTP and they withdraw before 1 days. However, in the case where pool has no collateral yet, (this can be due to all borrowers having fully paid back thier loans, or no users has added any collateral yet to take a loan) all lenders that wants to remove quote token are forced to pay a penalty regardless of the bucket they remove from.

Vulnerability Detail

In removeQuoteToken(), early withdrawal penalty is applied under this condition.

if (depositTime != 0 && block.timestamp - depositTime < 1 days) {
    if (removeParams.price > _ptp(poolState_.debt, poolState_.collateral)) {
        removedAmount_ = Maths.wmul(removedAmount_, Maths.WAD - _feeRate(poolState_.rate));
    }
}

It checks that the price bucket we are removing from should be less than or equal to the ptp. Otherwise, penalty is imposed. However, if poolState_.collateral = 0, the if condition will always be true as _ptp() is computed this way and returns 0 when collateral == 0.

function _ptp(
    uint256 debt_,
    uint256 collateral_
) pure returns (uint256 ptp_) {
    if (collateral_ != 0) ptp_ = Maths.wdiv(debt_, collateral_);
}

Impact

Lenders are forced to pay the early withdrawal penalty if pool has no collateral currently regardless of the buckets they are removing from.

Code Snippet

https://github.com/sherlock-audit/2023-01-ajna/blob/main/contracts/src/libraries/external/LenderActions.sol#L342-L343

Tool used

Manual Review

Recommendation

We should not impose a penalty when Lenders are removing quoteToken when there is currently 0 collateral.

hrishibhat commented 1 year ago

Considering this issue as a low as it seems like a design implementation suggestion.

yixxas commented 1 year ago

Escalate for 1 USDC

This reported issue is not a design implementation suggestion. We should only impose a fee when the bucket in which we are removing quote token from is more than the _ptp like how it is implemented and intended. However, due to how _ptp is calculated, when collateral == 0, a fee will be imposed irrespective of the bucket quote token is being removed from.

sherlock-admin commented 1 year ago

Escalate for 1 USDC

This reported issue is not a design implementation suggestion. We should only impose a fee when the bucket in which we are removing quote token from is more than the _ptp like how it is implemented and intended. However, due to how _ptp is calculated, when collateral == 0, a fee will be imposed irrespective of the bucket quote token is being removed from.

You've created a valid escalation for 1 USDC!

To remove the escalation from consideration: Delete your comment. To change the amount you've staked on this escalation: Edit your comment (do not create a new comment).

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

dmitriia commented 1 year ago

As I understand it, it is the design convention that ptp is agreed to be zero when there is no collateral.

Formula doesn't work in this case and the value should be set to something. Zero is kind of logical as when there is no activity yet the deposit liquidity is needed the most to start the operations.

mattcushman commented 1 year ago

Confirmed: we define ptp to be zero when there is no collateral, and the rest is a consequence of this. We would like to avoid making exceptions for edge cases just while the pool is small.

hrishibhat commented 1 year ago

Escalation Rejected

Based on the above comments from the Senior & Sponsor, this is an acceptable design choice.

sherlock-admin commented 1 year ago

Escalation Rejected

Based on the above comments from the Senior & Sponsor, this is an acceptable design choice.

This issue's escalations have been rejected!

Watsons who escalated this issue will have their escalation amount deducted from their next payout.