A public vault owner can lock and steal arbitrary collateralId by issuing a poisonous lien

[code423n4]

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)
)

[c4-judge]

Picodes marked the issue as duplicate of #565

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to QA (Quality Assurance)

[c4-judge]

This previously downgraded issue has been upgraded by Picodes

[c4-judge]

Picodes marked the issue as not a duplicate

[c4-judge]

Picodes marked the issue as duplicate of #19