code423n4 commented 1 year ago

Lines of code

https://github.com/GenerationSoftware/pt-v5-vault/tree/b1deb5d494c25f885c34c83f014c8a855c5e2749/src/Vault.sol#L566-L568

Vulnerability details

Impact

The output amount validation is not correct in Vault.liquidate(), so the method might accept invalid output amount and refuse valid output amount.

Proof of Concept

In Vault.liquidate(), there is a validation about the output share amount should be less than or equal to the liquidatable yield.

    uint256 _liquidableYield = _liquidatableBalanceOf(_tokenOut);
    if (_amountOut > _liquidableYield) 
      revert LiquidationAmountOutGTYield(_amountOut, _liquidableYield);

The liquidatable yield amount is in underlying asset token. So the comparison between the share amount and the underlying asset amount is not appropriate.

We could get share tokens from asset tokens via exchange rate. The vault gets _liquidableYield and mints _amountOut, so the correct asset amount equivalent to _amountOut of the share token will be _amountOut * exchange rate. The correct validation should use the asset amount and the current implementation is not correct when the exchange rate is not 1.

Tools Used

Manual Review

Recommended Mitigation Steps

We should use the underlying equivalent with the exchange rate for the validation.

Assessed type

Error

c4-judge commented 1 year ago

Picodes marked the issue as duplicate of #427

c4-judge commented 11 months ago

Picodes marked the issue as satisfactory