sherlock-audit / 2022-10-astaria-judging

6 stars 1 forks source link

__141345__ - Auction still exists even paid off the loan #285

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 2 years ago

141345

medium

Auction still exists even paid off the loan

Summary

The auction will not be cancelled even the loan is paid off. As a result, the collateral will still be liquidated. If the bid is very low, the lenders of other positions of the collateral could have fund loss.

Vulnerability Detail

Consider the following case: A collateral is taking 2 different loan, 1 for a private vault with duration of 15 days for $100, 1 for a public vault with duration of 30 days for $1,000. Say after 15 days, the loan is not paid back, it will be liquidated and put on auction.

Now, if the borrower pays back the first loan, but the auction won't be cancelled and still be alive. Then the collateral could be liquidated and the 2nd loan lender could suffer fund loss.

Impact

Some lenders could loss fund they do not deserve.

Code Snippet

The auction won't be changed even after paid off the 1st lien token loan. https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/LienToken.sol#L594-L649

Tool used

Manual Review

Recommendation

Cancel the auction is all the lien token loans are paid off.

IAmTurnipBoy commented 1 year ago

Escalate for 1 USDC

Impossible scenario. Lien can't be paid off after it's expired and it can't be liquidated unless it's expired. So if the vault is being liquidated for lien 1 they couldn't payoff lien 1.

Relevant lines showing you can't pay overdue lien: https://github.com/sherlock-audit/2022-10-astaria/blob/7d12a5516b7c74099e1ce6fb4ec87c102aec2786/src/LienToken.sol#L607-L610

sherlock-admin commented 1 year ago

Escalate for 1 USDC

Impossible scenario. Lien can't be paid off after it's expired and it can't be liquidated unless it's expired. So if the vault is being liquidated for lien 1 they couldn't payoff lien 1.

Relevant lines showing you can't pay overdue lien: https://github.com/sherlock-audit/2022-10-astaria/blob/7d12a5516b7c74099e1ce6fb4ec87c102aec2786/src/LienToken.sol#L607-L610

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.

Evert0x commented 1 year ago

Escalation accepted

sherlock-admin commented 1 year ago

Escalation accepted

This issue's escalations have been accepted!

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

141345 commented 1 year ago

Escalate for 1 USDC

Impossible scenario. Lien can't be paid off after it's expired and it can't be liquidated unless it's expired. So if the vault is being liquidated for lien 1 they couldn't payoff lien 1.

Relevant lines showing you can't pay overdue lien: https://github.com/sherlock-audit/2022-10-astaria/blob/7d12a5516b7c74099e1ce6fb4ec87c102aec2786/src/LienToken.sol#L607-L610

The lien can be paid through createBid() from AuctionHouse.sol:

// lib/astaria-gpl/src/AuctionHouse.sol
  function createBid(uint256 tokenId, uint256 amount) external override {
        // ...
118:     _handleIncomingPayment(tokenId, vaultPayment, address(msg.sender));
        // ...
    }

  function _handleIncomingPayment() internal {
296:           LIEN_TOKEN.makePayment(tokenId, payment, lien.position, payer);
        // ...
    }

// src/LienToken.sol
  function makePayment(uint256 collateralId, uint256 paymentAmount) public {
    _makePayment(collateralId, paymentAmount);
  }

  function _makePayment(uint256 collateralId, uint256 totalCapitalAvailable)
    internal
  {
    uint256[] memory openLiens = liens[collateralId];
    uint256 paymentAmount = totalCapitalAvailable;
    for (uint256 i = 0; i < openLiens.length; ++i) {
      uint256 capitalSpent = _payment(
        collateralId,
        uint8(i),
        paymentAmount,
        address(msg.sender)
      );
      paymentAmount -= capitalSpent;
    }
  }

Here because the address(msg.sender) is AUCTION_HOUSE, the following require can pass.

607:     require(
608:       block.timestamp < end || address(msg.sender) == address(AUCTION_HOUSE),
609:       "cannot pay off an expired lien"
610:     );

So this lien can be paid. After _payment(), the lien can be deleted,

// src/LienToken.sol
  function _payment() internal returns (uint256) {
        // ...
638:       //delete liens
639:       _deleteLienPosition(collateralId, position);
640:       delete lienData[lienId]; //full delete
641: 
642:       _burn(lienId);
        // ...
    }

Also astaria docs indicates:

The borrower may reclaim their NFT by paying the reserve amount of the LienTokens before a bid reaches that price.

However, in the case of multiple loans with different time frame for 1 NFT, the auction might not work as expected. The main point of this issue is 2 separate loans could influence each other, if the earlier one was on auction. This mechanism introduces potential loss for the other loan, but the risk should be isolated. The risks increase because of the liquidation rule.

sherlock-admin commented 1 year ago

Escalate for 1 USDC

Impossible scenario. Lien can't be paid off after it's expired and it can't be liquidated unless it's expired. So if the vault is being liquidated for lien 1 they couldn't payoff lien 1.

Relevant lines showing you can't pay overdue lien: https://github.com/sherlock-audit/2022-10-astaria/blob/7d12a5516b7c74099e1ce6fb4ec87c102aec2786/src/LienToken.sol#L607-L610

The lien can be paid through createBid() from AuctionHouse.sol:

// lib/astaria-gpl/src/AuctionHouse.sol
  function createBid(uint256 tokenId, uint256 amount) external override {
        // ...
118:     _handleIncomingPayment(tokenId, vaultPayment, address(msg.sender));
        // ...
    }

  function _handleIncomingPayment() internal {
296:           LIEN_TOKEN.makePayment(tokenId, payment, lien.position, payer);
        // ...
    }

// src/LienToken.sol
  function makePayment(uint256 collateralId, uint256 paymentAmount) public {
    _makePayment(collateralId, paymentAmount);
  }

  function _makePayment(uint256 collateralId, uint256 totalCapitalAvailable)
    internal
  {
    uint256[] memory openLiens = liens[collateralId];
    uint256 paymentAmount = totalCapitalAvailable;
    for (uint256 i = 0; i < openLiens.length; ++i) {
      uint256 capitalSpent = _payment(
        collateralId,
        uint8(i),
        paymentAmount,
        address(msg.sender)
      );
      paymentAmount -= capitalSpent;
    }
  }

Here because the address(msg.sender) is AUCTION_HOUSE, the following require can pass.

607:     require(
608:       block.timestamp < end || address(msg.sender) == address(AUCTION_HOUSE),
609:       "cannot pay off an expired lien"
610:     );

So this lien can be paid. After _payment(), the lien can be deleted,

// src/LienToken.sol
  function _payment() internal returns (uint256) {
        // ...
638:       //delete liens
639:       _deleteLienPosition(collateralId, position);
640:       delete lienData[lienId]; //full delete
641: 
642:       _burn(lienId);
        // ...
    }

Also astaria docs indicates:

The borrower may reclaim their NFT by paying the reserve amount of the LienTokens before a bid reaches that price.

However, in the case of multiple loans with different time frame for 1 NFT, the auction might not work as expected. The main point of this issue is 2 separate loans could influence each other, if the earlier one was on auction. This mechanism introduces potential loss for the other loan, but the risk should be isolated. The risks increase because of the liquidation rule.

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.

Evert0x commented 1 year ago

Escalation rejected. After looking into it we still think the submission fails to demonstrate the issue in a clear manner.

sherlock-admin commented 1 year ago

Escalation rejected. After looking into it we still think the submission fails to demonstrate the issue in a clear manner.

This issue's escalations have been rejected!

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