settleAuction() Check for status errors

[code423n4]

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/CollateralToken.sol#L526-L534

Vulnerability details

Impact

ClearingHouse.safeTransferFrom() to execute successfully even if there is no bid

Proof of Concept

settleAuction is called at the end of the auction and will check if the status is legal

  function settleAuction(uint256 collateralId) public {
    if (
      s.collateralIdToAuction[collateralId] == bytes32(0) &&
      ERC721(s.idToUnderlying[collateralId].tokenContract).ownerOf(
        s.idToUnderlying[collateralId].tokenId
      ) !=
      s.clearingHouse[collateralId]
    ) {
      revert InvalidCollateralState(InvalidCollateralStates.NO_AUCTION);
    }

This check seems to be miswritten，The normal logic would be

s.collateralIdToAuction[collateralId] == bytes32(0) || ERC721(s.idToUnderlying[collateralId].tokenContract).ownerOf(
        s.idToUnderlying[collateralId].tokenId
      ) == s.clearingHouse[collateralId]

This causes ClearingHouse.safeTransferFrom() to execute successfully even if there is no bid

Tools Used

Recommended Mitigation Steps

  function settleAuction(uint256 collateralId) public {
    if (
-     s.collateralIdToAuction[collateralId] == bytes32(0) &&
-    ERC721(s.idToUnderlying[collateralId].tokenContract).ownerOf(
-        s.idToUnderlying[collateralId].tokenId
-      ) !=
-      s.clearingHouse[collateralId]
+      s.collateralIdToAuction[collateralId] == bytes32(0) || 
+       ERC721(s.idToUnderlying[collateralId].tokenContract).ownerOf(s.idToUnderlying[collateralId].tokenId
+      ) == 
+      s.clearingHouse[collateralId]
    ) {
      revert InvalidCollateralState(InvalidCollateralStates.NO_AUCTION);
    }

[c4-judge]

Picodes marked the issue as primary issue

[c4-sponsor]

androolloyd marked the issue as sponsor confirmed

[c4-judge]

Picodes marked the issue as selected for report

[c4-judge]

Picodes marked the issue as satisfactory

[Picodes]

Keeping medium severity despite the lack of clear impact, the lack of clear impact being due to flaws in the flow before these lines