Jeiwan - Cancelling an auction doesn't repay the entire debt and doesn't unlock collateral

[sherlock-admin]

Jeiwan

high

Cancelling an auction doesn't repay the entire debt and doesn't unlock collateral

Summary

Cancelling an auction doesn't repay the entire debt and doesn't unlock collateral

Vulnerability Detail

When cancelling an auction, the reservePrice is expected to be paid (AuctionHouse.sol#L220). The price is the sum of outstanding debts of all liens (AuctionHouse.sol#L73-L77, LienToken.sol#L207-L213). However, when the repayment is made, an initiator fee is subtracted from the amount being repaid (AuctionHouse.sol#L265-L276). This reduces the total amount being repaid (AuctionHouse.sol#L276) and leaves at least one lien unrepaid (AuctionHouse.sol#L281-L298) (totalAmount won't be enough to repay all liens). The auction, however, will still be removed (AuctionHouse.sol#L223) and the owner won't be able to release their collateral due to the unrepaid lien.

Impact

After a borrower has cancelled an auction on their collateral and repaid the full amount of the debt, they won't be able to release the collateral since at least one lien won't be repaid. Anyone will be able to start a new auction, and since the total loan will be mostly repaid, the reserve price of the new auction will be low and it'll be cheap to create a bid that's higher than the reserve price, thus disallowing the borrower to cancel the auction (AuctionHouse.sol#L215). The borrower basically loses the amount the paid to cancel the auction.

Code Snippet

CollateralToken.sol#L331-L335:

function cancelAuction(uint256 tokenId) external onlyOwner(tokenId) {
  require(AUCTION_HOUSE.auctionExists(tokenId), "Auction doesn't exist");

  AUCTION_HOUSE.cancelAuction(tokenId, msg.sender);
}

AuctionHouse.sol#L210:

function cancelAuction(uint256 auctionId, address canceledBy)
  external
  requiresAuth
{
  require(
    auctions[auctionId].currentBid < auctions[auctionId].reservePrice,
    "cancelAuction: Auction is at or above reserve"
  );
  _handleIncomingPayment(
    auctionId,
    auctions[auctionId].reservePrice, // @audit borrower must pay this amount to cancel the auction
    canceledBy
  );
  _cancelAuction(auctionId);
}

AuctionHouse.sol#L253:

function _handleIncomingPayment(
  uint256 tokenId,
  uint256 transferAmount,
  address payer
) internal {
  require(transferAmount > uint256(0), "cannot send nothing");

  Auction storage auction = auctions[tokenId];

  //fee is in percent
  //muldiv?
  //        uint256 initiatorPayment = (transferAmount * auction.initiatorFee) / 100;
  uint256 initiatorPayment = transferAmount.mulDivDown(
    auction.initiatorFee,
    100
  ); //maybe consider making protocl computed like other fees

  TRANSFER_PROXY.tokenTransferFrom(
    weth,
    payer,
    auction.initiator,
    initiatorPayment
  );
  // @audit total repayment amount gets reduced => all liens won't be repaid
  transferAmount -= initiatorPayment;

  uint256[] memory liens = LIEN_TOKEN.getLiens(tokenId);
  uint256 totalLienAmount = 0;
  if (liens.length > 0) {
    for (uint256 i = 0; i < liens.length; ++i) {
      uint256 payment;
      uint256 lienId = liens[i];

      ILienToken.Lien memory lien = LIEN_TOKEN.getLien(lienId);

      if (transferAmount >= lien.amount) {
        payment = lien.amount;
        transferAmount -= payment;
      } else {
        payment = transferAmount;
        transferAmount = 0;
      }

      if (payment > 0) {
        LIEN_TOKEN.makePayment(tokenId, payment, lien.position, payer);
      }
    }
  } else {
    TRANSFER_PROXY.tokenTransferFrom(
      weth,
      payer,
      COLLATERAL_TOKEN.ownerOf(tokenId),
      transferAmount
    );
  }
}

CollateralToken.sol#L204-L214:

modifier releaseCheck(uint256 collateralId) {
  require(
    // @audit collateral can be released only when all liens were repaid
    uint256(0) == LIEN_TOKEN.getLiens(collateralId).length &&
      !AUCTION_HOUSE.auctionExists(collateralId),
    "must be no liens or auctions to call this"
  );
  _;
}

function releaseToAddress(uint256 collateralId, address releaseTo)
  public
  releaseCheck(collateralId)
{
  //check liens
  require(
    msg.sender == ownerOf(collateralId),
    "You don't have permission to call this"
  );
  _releaseToAddress(collateralId, releaseTo);
}

Tool used

Manual Review

Recommendation

Consider including the initiator fee in the amount required to cancel an auction.

Duplicate of #199