HonorLt - _validateCommitment validations

[sherlock-admin]

HonorLt

low

_validateCommitment validations

Summary

I've identified a few improvements when validating commitments. See the details.

Vulnerability Detail

1) When validating the msg.sender it could also accept isApprovedForAll:

    address operator = ERC721(COLLATERAL_TOKEN()).getApproved(collateralId);

    address holder = ERC721(COLLATERAL_TOKEN()).ownerOf(collateralId);

    if (msg.sender != holder) {
      require(msg.sender == operator, "invalid request");
    }

2) When rate === maxInterestRate should also be ok (<=):

    require(
      ld.rate < IAstariaRouter(ROUTER()).maxInterestRate(),
      "Vault._validateCommitment(): Rate is above maximum"
    );

Impact

No serious impact, just some technical improvements.

Code Snippet

https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/VaultImplementation.sol#L152-L165

https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/VaultImplementation.sol#L201-L204

Tool used

Manual Review

Recommendation

Consider making proposed changes.