sherlock-audit / 2022-10-astaria-judging

6 stars 1 forks source link

bin2chen - _payment() maybe overpayment #278

Closed sherlock-admin closed 2 years ago

sherlock-admin commented 2 years ago

bin2chen

high

_payment() maybe overpayment

Summary

LienToken#makePayment can pass the amount of payment, if the parameter paymentAmount is greater than lien.amount, it may be overpaid

Vulnerability Detail

in #_payment() , when lien.amount<= paymentAmount, the smallest amount "min(lien.amount,paymentAmount)" should be taken to pay

  function _payment(
    uint256 collateralId,
    uint8 position,
    uint256 paymentAmount,
    address payer
  ) internal returns (uint256) {
.....
    if (lien.amount > paymentAmount) {
      lien.amount -= paymentAmount;

    } else {
      ...
       /***** don't set paymentAmount =  lien.amount ********/
      _burn(lienId);
    }
    TRANSFER_PROXY.tokenTransferFrom(WETH, payer, payee, paymentAmount); //
}

Impact

lost eth

Code Snippet

https://github.com/sherlock-audit/2022-10-astaria/blob/main/src/LienToken.sol#L630-L645

Tool used

Manual Review

Recommendation

  function _payment(
    uint256 collateralId,
    uint8 position,
    uint256 paymentAmount,
    address payer
  ) internal returns (uint256) {
.....
    if (lien.amount > paymentAmount) {
      lien.amount -= paymentAmount;

    } else {
      ...
        _deleteLienPosition(collateralId, position); 
        delete lienData[lienId]; //full delete 

         _burn(lienId); 

+       paymentAmount =  lien.amount;
    }

    TRANSFER_PROXY.tokenTransferFrom(WETH, payer, payee, paymentAmount); 

    emit Payment(lienId, paymentAmount);
    return paymentAmount;
}

Duplicate of #28