KupiaSec - `TpdaLiquidationPair.swapExactAmountOut()` calculates `swapAmountIn` incorrectly

[sherlock-admin2]

KupiaSec

high

TpdaLiquidationPair.swapExactAmountOut() calculates swapAmountIn incorrectly

Summary

In swapExactAmountOut(), swapAmountIn is calculated without using the _amountOut param.

Vulnerability Detail

swapExactAmountOut() is designed to sell one token for another at an auction price which is inversely proportional to the time since the last auction.

_computePrice() returns the auction price according to the elapsed time as intended.

But swapExactAmountOut() uses the auction price as swapAmountIn wrongly.

File: TpdaLiquidationPair.sol
122:     function swapExactAmountOut(
123:         address _receiver,
124:         uint256 _amountOut,
125:         uint256 _amountInMax,
126:         bytes calldata _flashSwapData
127:     ) external returns (uint256) {
128:         if (_receiver == address(0)) {
129:             revert ReceiverIsZero();
130:         }
131: 
132:         uint192 swapAmountIn = _computePrice(); //@audit don't use _amountOut
133: 
134:         if (swapAmountIn > _amountInMax) {
135:             revert SwapExceedsMax(_amountInMax, swapAmountIn);
136:         }

...
190:     function _computePrice() internal view returns (uint192) {
191:         uint256 elapsedTime = block.timestamp - lastAuctionAt;
192:         if (elapsedTime == 0) {
193:             return type(uint192).max;
194:         }
195:         uint192 price = uint192((targetAuctionPeriod * lastAuctionPrice) / elapsedTime);
196: 
197:         if (price < MIN_PRICE) {
198:             price = MIN_PRICE;
199:         }
200: 
201:         return price;
202:     }

So the same swapAmountIn will be applied for any _amountOut and users can swap out as much as possible(up to _availableBalance).

Even if it's an intended design to use _computePrice() as swapAmountIn, the swap ratio (= amountIn/amountOut) would be decreasing irregularly because tokenOut balance of the liquidation pair can be increased at any time.

Impact

swapExactAmountOut() will use the same swapAmountIn for any amountOut to swap.

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/1aa1b8c028b659585e4c7a6b9b652fb075f86db3/pt-v5-tpda-liquidator/src/TpdaLiquidationPair.sol#L132

Tool used

Manual Review

Recommendation

swapExactAmountOut() should calculate swapAmountIn according to the current auction price and _amountOut.

 function swapExactAmountOut(
 address _receiver,
 uint256 _amountOut,
 uint256 _amountInMax,
 bytes calldata _flashSwapData
 ) external returns (uint256) {
 if (_receiver == address(0)) {
 revert ReceiverIsZero();
 }

-        uint192 swapAmountIn = _computePrice();
+        uint192 swapPrice = _computePrice();
+        uint192 swapAmountIn = _amountOut * swapPrice / 1e18;

 if (swapAmountIn > _amountInMax) {
 revert SwapExceedsMax(_amountInMax, swapAmountIn);
 }

 lastAuctionAt = uint64(block.timestamp);
-        lastAuctionPrice = swapAmountIn;
+        lastAuctionPrice = swapPrice;

[sherlock-admin3]

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

infect3d commented:

expected behavior of the auction

[nevillehuang]

Invalid, sponsor comments:

• this is intended design; the TPDA auction offers the entire amount out for an amount in that drops over time
• amountIn drops over time, amountOut increases (or stays the same) over time, the auction occurs when the common value of these two values meet