peanuts - LenderAuctions#_removeMaxCollateral() does not check bucket's backruptcyTime.

[github-actions[bot]]

peanuts

medium

LenderAuctions#_removeMaxCollateral() does not check bucket's backruptcyTime.

Summary

If bucket is insolvent, lender should not be able to call removeMaxCollateral().

Vulnerability Detail

The function _removeMaxCollateral() removes the max collateral amount from a given bucket index. If the bucket.bankruptcyTime is less than the lender.depositTime, then the lenderLpBalance = lender.lps. However, if the bucket is bankrupted, then nothing happens.

    function _removeMaxCollateral(
        mapping(uint256 => Bucket) storage buckets_,
        DepositsState storage deposits_,
        uint256 maxAmount_,
        uint256 index_
    ) internal returns (uint256 collateralAmount_, uint256 lpAmount_) {
        Bucket storage bucket = buckets_[index_];

        uint256 bucketCollateral = bucket.collateral;
        if (bucketCollateral == 0) revert InsufficientCollateral(); // revert if there's no collateral in bucket

        Lender storage lender = bucket.lenders[msg.sender];

        uint256 lenderLpBalance;

        if (bucket.bankruptcyTime < lender.depositTime) lenderLpBalance = lender.lps;
        if (lenderLpBalance == 0) revert NoClaim();                  // revert if no LP to redeem

Impact

Bucket can still be withdrawn despite insolvency.

Code Snippet

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

Tool used

Manual Review

Recommendation

Make sure the lender cannot add / remove tokens if bucket is bankrupted.

[yixxas]

Escalate for 1 USDC

This report is not a valid dupe of #133. #133 talks about a missing bankruptcy logic in removeCollateral(). This report however is talking about a completely different issue.

[sherlock-admin]

Escalate for 1 USDC

This report is not a valid dupe of #133. #133 talks about a missing bankruptcy logic in removeCollateral(). This report however is talking about a completely different issue.

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]

Escalate for 50 USDC I'm sorry, but this looks invalid. There is no issue with _removeMaxCollateral's if (bucket.bankruptcyTime < lender.depositTime) lenderLpBalance = lender.lps logic, it is common for the protocol: if lender deposited after bankruptcy time, then the deposit is active, otherwise it's not, lender's position is defaulted and the function reverts:

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

        uint256 lenderLpBalance;

        if (bucket.bankruptcyTime < lender.depositTime) lenderLpBalance = lender.lps;
        if (lenderLpBalance == 0) revert NoClaim();                  // revert if no LP to redeem

[sherlock-admin]

Escalate for 50 USDC I'm sorry, but this looks invalid. There is no issue with _removeMaxCollateral's if (bucket.bankruptcyTime < lender.depositTime) lenderLpBalance = lender.lps logic, it is common for the protocol: if lender deposited after bankruptcy time, then the deposit is active, otherwise it's not, lender's position is defaulted and the function reverts:

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

        uint256 lenderLpBalance;

        if (bucket.bankruptcyTime < lender.depositTime) lenderLpBalance = lender.lps;
        if (lenderLpBalance == 0) revert NoClaim();                  // revert if no LP to redeem

You've created a valid escalation for 50 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.

[hrishibhat]

Escalation accepted

This issue is not a valid dupe of #133 and not a valid issue on its own as pointed out by the above escalation.

[sherlock-admin]

Escalation accepted

This issue is not a valid dupe of #133 and not a valid issue on its own as pointed out by the above escalation.

This issue's escalations have been accepted!

Contestants' payouts and scores will be updated according to the changes made on this issue.