code423n4 commented 1 year ago

Lines of code

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

Vulnerability details

Impact

Illegal liquidationInitialAsk and maxPotentialDebt may result in bids amount do not cover the debt

Proof of Concept

With the current implementation, the first LienToken does not check liquidationInitialAsk and maxPotentialDebt

  function _appendStack(
    LienStorage storage s,
    Stack[] memory stack,
    Stack memory newSlot
  ) internal returns (Stack[] memory newStack) {
    if (stack.length >= s.maxLiens) {
      revert InvalidState(InvalidStates.MAX_LIENS);
    }

    newStack = new Stack[](stack.length + 1);
    newStack[stack.length] = newSlot;

    uint256 potentialDebt = _getOwed(newSlot, newSlot.point.end);
    //****@audit check in for () ,so   first LienToken not check  liquidationInitialAsk****//
    for (uint256 i = stack.length; i > 0; ) {
      uint256 j = i - 1;
      newStack[j] = stack[j];
      if (block.timestamp >= newStack[j].point.end) {
        revert InvalidState(InvalidStates.EXPIRED_LIEN);
      }

      unchecked {
        potentialDebt += _getOwed(newStack[j], newStack[j].point.end);
      }

      if (potentialDebt > newStack[j].lien.details.liquidationInitialAsk) {
        revert InvalidState(InvalidStates.INITIAL_ASK_EXCEEDED);
      }

      unchecked {
        --i;
      }
    }
    if (
      //****@audit  need stack.length>0, so first LienToken will Ignore ****//
      stack.length > 0 && potentialDebt > newSlot.lien.details.maxPotentialDebt
    ) {
      revert InvalidState(InvalidStates.DEBT_LIMIT);
    }
  }

Tools Used

Recommended Mitigation Steps

need check when first LienToken

  function _appendStack(
    LienStorage storage s,
    Stack[] memory stack,
    Stack memory newSlot
  ) internal returns (Stack[] memory newStack) {
...
    uint256 potentialDebt = _getOwed(newSlot, newSlot.point.end);

+    if (potentialDebt > newSlot.lien.details.liquidationInitialAsk) {
+      revert InvalidState(InvalidStates.INITIAL_ASK_EXCEEDED);
+    }

...
    if (
-     stack.length > 0 && potentialDebt > newSlot.lien.details.maxPotentialDebt
+    potentialDebt > newSlot.lien.details.maxPotentialDebt
    ) {
      revert InvalidState(InvalidStates.DEBT_LIMIT);
    }

androolloyd commented 1 year ago

The first liens don't need to check max potential debt because there's no debt in existence.

Max potential debt doesn't It include the value of the new lien only the value of liens already open.

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #147