ydlee - `TpdaLiquidationPair.swapExactAmountOut` does not refund excess inTokens if not called from `TpdaLiquidationRouter`.

[sherlock-admin3]

ydlee

medium

TpdaLiquidationPair.swapExactAmountOut does not refund excess inTokens if not called from TpdaLiquidationRouter.

Summary

If TpdaLiquidationPair.swapExactAmountOut is not called from TpdaLiquidationRouter, the amount of tokens being swapped in must be sent to the target before calling this function. The amount of tokens sent in advance may exceed the actual amount needed for the swapping, but the excess tokens are not returned to user, causing the user to suffer a loss of tokens.

Vulnerability Detail

If TpdaLiquidationPair.swapExactAmountOut is called from liquidation router, the _flashSwapData parameter will hold user's address, and the right amount of in tokens needed for the swapping will be transfered to the target in the flashSwapCallback (L154). This calling path is fine.

If TpdaLiquidationPair.swapExactAmountOut is NOT called from liquidation router, the user should transfer some amount of in tokens to the target in advance, and that amount (possibly _amountInMax) usually exceeds the actual amount needed to ensure a successful swap (L134). However, the excess in tokens are not refunded to user after the swapping, causing the user to suffer a loss of tokens.

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();
133:
134:@>      if (swapAmountIn > _amountInMax) {
135:            revert SwapExceedsMax(_amountInMax, swapAmountIn);
136:        }
137:
138:        lastAuctionAt = uint64(block.timestamp);
139:        lastAuctionPrice = swapAmountIn;
140:
141:        uint256 availableOut = _availableBalance();
142:        if (_amountOut > availableOut) {
143:            revert InsufficientBalance(_amountOut, availableOut);
144:        }
145:
146:        bytes memory transferTokensOutData = source.transferTokensOut(
147:            msg.sender,
148:            _receiver,
149:            address(_tokenOut),
150:            _amountOut
151:        );
152:
153:        if (_flashSwapData.length > 0) {
154:@>          IFlashSwapCallback(_receiver).flashSwapCallback(
155:                msg.sender,
156:                swapAmountIn,
157:                _amountOut,
158:                _flashSwapData
159:            ); // @audit in tokens are transfered to target if called from liquidation router
160:        }
161:
162:@>      source.verifyTokensIn(address(_tokenIn), swapAmountIn, transferTokensOutData);
163:
164:        emit SwappedExactAmountOut(msg.sender, _receiver, _amountOut, _amountInMax, swapAmountIn, _flashSwapData);
165:
166:        return swapAmountIn;
167:    }

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

Impact

Users may suffer a loss of tokens if TpdaLiquidationPair.swapExactAmountOut is not called from liquidation router.

Code Snippet

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

Tool used

Manual Review

Recommendation

Refund the excess in tokens if TpdaLiquidationPair.swapExactAmountOut is not called from liquidation router.

[sherlock-admin2]

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

infect3d commented:

user input error are invalid

[nevillehuang]

Invalid, user is not expected to interact with liquidation pair directly, but instead go through the router, so this would be user input error since the necessary safety checks is present in the router.