code423n4 commented 1 year ago

Lines of code

https://github.com/code-423n4/2023-01-astaria/blob/1bfc58b42109b839528ab1c21dc9803d663df898/src/LienToken.sol#L550-L560

Vulnerability details

Vulnerability Detail

function LienToken.getAmountOwingAtLiquidation() is used to retrieve a specific point by its lienId. Its implementation is as follow:

  /**
   * @notice Retrieves a specific point by its lienId.
   * @param stack the Lien to compute a point for
   */
  function getAmountOwingAtLiquidation(ILienToken.Stack calldata stack)
    public
    view
    returns (uint256)
  {
    return
      _loadLienStorageSlot()
        .auctionData[stack.lien.collateralId]
        .stack[stack.point.lienId] 
        .amountOwed;
  }

As we can see this function will return the amountOwed of the auction data at position stack.point.lienId of stack. Since stack.point.lienId in other functions like LienToken.validateLien() is calculated as lienId = uint256(keccak256(abi.encode(lien))), it can be much larger than the stack size. So obviously this stack.point.lienId doesn't have the same purpose as the lienId the protocol used. It will make users who want to integrate with Astaria into their contracts confuse about the input parameter when using this function. It can incur the loss for the user / partner when use the function in a wrong way.

Impact

Misunderstood by the partner / user when use the function.
Function doesn't work as expected

Tool used

Manual Review

Recommendation

Modify function as follow:

 /**
   * @notice Retrieves a specific point by its lienId.
   * @param stack the Lien to compute a point for
   */
  function getAmountOwingAtLiquidation(uint collateralId, uint position)
    public
    view
    returns (uint256)
  {
    return
      _loadLienStorageSlot()
        .auctionData[collateralId]
        .stack[position] 
        .amountOwed;
  }