sherlock-audit / 2024-05-pooltogether-judging

8 stars 4 forks source link

evmboi32 - Vault can auction off yield for an incorrect price #72

Closed sherlock-admin3 closed 2 months ago

sherlock-admin3 commented 3 months ago

evmboi32

medium

Vault can auction off yield for an incorrect price

Summary

The lastAuctionAt variable is set incorrectly.

Vulnerability Detail

When deploying the LiquidationPair the lastAuctionAt is set to the block.timestamp in the constructor. This indicates that the auction starts as soon as the LiquidationPair is deployed. If the pair is deployed a lot earlier than the first yield auction happens on the vault, it could auction off yield for a very low price.

The amount of tokens that a user needs to provide will lower as time passes on.

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-tpda-liquidator/src/TpdaLiquidationPair.sol#L190-L202

In case of a very large elapsedTime the price could be too low for the amount of yield the liquidator will receive. This will cause a loss for the vault as it will sell the yield for less than it is worth. This scenario can happen only for the first yield auction on the vault if the owner of the vault sets the predeployed liquidation pair on the vault too late. (eg. deploy liq pair and after a day set it as a liq pair for the vault)

Impact

Yield will be auctioned off for less than its worth.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/main/pt-v5-tpda-liquidator/src/TpdaLiquidationPair.sol#L190-L202

Tool used

Manual Review

Recommendation

Only start the auction on the vault when the first deposit happens on a vault, or when the liquidationPair is set on a vault.

Coded POC

Add this test to the TpdaLiquidationPair.t.sol and run with forge test --match-path ./test/TpdaLiquidationPair.t.sol -vvv

function test_computePrice_Test() public {
    vm.warp(block.timestamp + 10);
    console2.log("Price at the start:", pair.computeExactAmountIn(0));

    vm.warp(block.timestamp + 1 hours);
    console2.log("Price after 1 hour:", pair.computeExactAmountIn(0));

    vm.warp(block.timestamp + 1 hours);
    console2.log("Price after 1 day :", pair.computeExactAmountIn(0));
}
[PASS] test_computePrice_Test() (gas: 19508)
Logs:
  Price at the start: 3600000000000000000
  Price after 1 hour: 9972299168975069
  Price after 1 day : 399955560493278
sherlock-admin2 commented 2 months ago

1 comment(s) were left on this issue during the judging contest.

infect3d commented:

can be solved by carefully chosing the constructor lastAuctionPrice parameter

nevillehuang commented 2 months ago

Invalid, sponsor comments:

  • Invalid since yield will accrue on the vault until it meets the auction price of the LP; if the price is very low, then the auction will not need much yield to accrue before it is profitable