commitToLiens via AstariaRouter will not work

Summary

I assume that calling commitToLiens on AstariaRouter should revert. Please see the details.

Vulnerability Detail

When committing to liens via a router, first it transfers the NFT from the user directly to the COLLATERAL_TOKEN. This should trigger onERC721Received and mint the corresponding collateral token. The token is minted directly to the user, not the operator (router):

      address depositFor = operator_;

      if (operator_ != from_) {
        depositFor = from_;
      }

      _mint(depositFor, collateralId);

However, it seems that the router fails to account for that and later tries to _returnCollateral to the user. This should revert the tx because the router does not own the collateral.

    COLLATERAL_TOKEN.transferFrom(address(this), receiver, collateralId);

Impact

This makes it impossible to commit via router and the user needs to explicitly execute several actions in a direct sequence.

Code Snippet

https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/AstariaRouter.sol#L244-L274

https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/CollateralToken.sol#L280-L286

https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/AstariaRouter.sol#L558-L561

https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/AstariaRouter.sol#L589-L591

Tool used

Manual Review

Recommendation

You do not need to return the collateral if it already belongs to the user or the collateral token should be minted to the operator not the user depending on intentions.

Duplicate of #204

sherlock-audit / 2022-10-astaria-judging

HonorLt - commitToLiens via AstariaRouter will not work #281