Astaria contest - cergyk's results

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/ClearingHouse.sol#L169-L178 https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/ClearingHouse.sol#L123

Vulnerability details

Impact

ClearingHouse implements the ERC1155 safeTransferFrom hook for seaport to call when it resolves considerations. Only it does not check that the call comes from seaport. A malicious user can call this function encoding the settlement token of their choice and redeem the underlying NFT, providing that an auction is taking place for it (checked in Collateral.settleAuction)

Proof of Concept

Links provided in relevant section

Tools Used

Manual review

Recommended Mitigation Steps

Should either check that initiator is seaport, or that settlement token is the one required by the auction.

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/VaultImplementation.sol#L238-L241

Vulnerability details

Impact

Since any vault can issue a lien to any collateralId (using the case receiver == holder), a malicious vault owner can lock any collateralId not having liens by issuing a malicious lien.

Here is the detailed scenario: An innocent user A deposits their NFT in the CollateralToken contract, mints a token with id collateralId.

If user A did not commit to any lien yet, malicious user B creates a public vault, and issues a lien to A for collateralId, denominated in a ERC20 token controlled by B.

B locks the transfer function on their ERC20 token, until auction, during which B is the only one to be able to transfer the ERC20, and thus obtaining the right to claim the collateral.

Proof of Concept

Tools Used

Manual review

Recommended Mitigation Steps

The condition for validity should be (always checking msg.sender):

if (
      (msg.sender != holder) &&
        (msg.sender != operator) &&
        !CT.isApprovedForAll(holder, msg.sender)
)