Laksmana - Missing Validation of TpdaLiquidationPair

[sherlock-admin2]

Laksmana

medium

Missing Validation of TpdaLiquidationPair

Summary

TpdaLiquidationPairFactory#createPair didn't check the _tokenIn and _tokenOut, as a result TpdaLiquidationPair cannot be used.

Vulnerability Detail

The function TpdaLiquidationPair Factory#createPair is a function to create a TpdaLiquidationPair contract, but unfortunately there is no check for tokenIn.

 function createPair(
        ILiquidationSource _source,
        address _tokenIn,
        address _tokenOut,
        uint64 _targetAuctionPeriod,
        uint192 _targetAuctionPrice,
        uint256 _smoothingFactor
    ) external returns (TpdaLiquidationPair) {
        TpdaLiquidationPair _liquidationPair = new TpdaLiquidationPair(
            _source,
            _tokenIn,
            _tokenOut,
            _targetAuctionPeriod,
            _targetAuctionPrice,
            _smoothingFactor
        );

Q: Why should there be a _tokenIn check ?

A: Look at this TpdaLiquidationPair#swapExactAmountOut function.

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

        uint192 swapAmountIn = _computePrice();

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

        lastAuctionAt = uint64(block.timestamp);
        lastAuctionPrice = swapAmountIn;

        uint256 availableOut = _availableBalance();
        if (_amountOut > availableOut) {
            revert InsufficientBalance(_amountOut, availableOut);
        }

        bytes memory transferTokensOutData = source.transferTokensOut(
            msg.sender,
            _receiver,
            address(_tokenOut),
            _amountOut
        );

        if (_flashSwapData.length > 0) {
            IFlashSwapCallback(_receiver).flashSwapCallback(
                msg.sender,
                swapAmountIn,
                _amountOut,
                _flashSwapData
            );
        }

        source.verifyTokensIn(address(_tokenIn), swapAmountIn, transferTokensOutData);

        emit SwappedExactAmountOut(msg.sender, _receiver, _amountOut, _amountInMax, swapAmountIn, _flashSwapData);

        return swapAmountIn;
    }

that function call source.transferTokensOut and source.verifyTokensIn which is the PrizeVault#transferTokensOut and PrizeVault#verifyTokensIn but look this.

  function transferTokensOut(
        address /* sender */,
        address _receiver,
        address _tokenOut,
        uint256 _amountOut
    ) external virtual onlyLiquidationPair returns (bytes memory) {
.............................................
   if (_tokenOut == address(_asset)) {
            _enforceMintLimit(_totalDebtBefore, _yieldFee);
            _withdraw(_receiver, _amountOut);
        } else if (_tokenOut == address(this)) {
            _enforceMintLimit(_totalDebtBefore, _amountOut + _yieldFee);
            _mint(_receiver, _amountOut);
        } else {
            revert LiquidationTokenOutNotSupported(_tokenOut);
        }

///////////////////////////////////////////////////////////////////

  function verifyTokensIn(
        address _tokenIn,
        uint256 _amountIn,
        bytes calldata /* transferTokensOutData */
    ) external onlyLiquidationPair {
        address _prizeToken = address(prizePool.prizeToken());
        if (_tokenIn != _prizeToken) {
            revert LiquidationTokenInNotPrizeToken(_tokenIn, _prizeToken);
        }

        prizePool.contributePrizeTokens(address(this), _amountIn);
    }

See If address _tokenOut is not equal to address _asset or _source it will revert. also if (_tokenIn != _prizeToken), so when verifyTokensIn is triggred but the address of _tokenIn is not equal to address _prizeToken it will revert.

As a result, TpdaLiquidationPair cannot be used.

Impact

• When TpdaLiquidationPair is created and address _tokenOut is not equal to address _asset or _source it will succeed, but TpdaLiquidationPair will be a useless contract because it cannot call swapExactAmountOut. OR
• When TpdaLiquidationPair is created and _tokenIn is not equal to prizeToken it will succeed, but TpdaLiquidationPair will be a useless contract because it cannot call swapExactAmountOut.

POC

• Paste POC code below in file PrizeVaultFactory.t.sol
• Run test :
• forge test -vvvv --match-test test_TpdaLiquidationPair_ErrorOf_tokenOut
• forge test -vvvv --match-test test_TpdaLiquidationPair_ErrorOf_tokenIn

 function test_TpdaLiquidationPair_ErrorOf_tokenOut() public {
        uint64 targetAuctionPeriod = 1 hours;
        uint192 auctionTargetPrice = 1e18;
        uint256 smoothing = 0.1e18;

        TpdaLiquidationPairFactory tpdafactory = new TpdaLiquidationPairFactory();
        PrizeVault _vault;

        address tokenIn = makeAddr("tokenIn");
        address tokenOut = makeAddr("tokenOut");
        address receiver = makeAddr("receiver");

        asset.mint(address(this), yieldBuffer);
        asset.approve(address(vaultFactory), yieldBuffer);

        _vault = vaultFactory.deployVault(
            name,
            symbol,
            yieldVault,
            PrizePool(address(prizePool)),
            claimer,
            address(this),
            yieldFeePercentage,
            yieldBuffer,
            owner
        );
        asset.mint(address(_vault), 1234e18 * 2);

        TpdaLiquidationPair liquidationPair = tpdafactory.createPair(
            _vault,
            tokenIn,
            tokenOut,
            targetAuctionPeriod,
            auctionTargetPrice,
            smoothing
        );

        vm.prank(owner);
        _vault.setLiquidationPair(address(liquidationPair));

        uint256 balance = 1234e18;
        vm.mockCall(
            address(_vault),
            abi.encodeWithSelector(_vault.liquidatableBalanceOf.selector, address(tokenOut)),
            abi.encode(balance)
        );
        vm.mockCall(
            address(_vault),
            abi.encodeWithSelector(
                _vault.transferTokensOut.selector,
                address(liquidationPair),
                receiver,
                address(tokenOut),
                balance
            ),
            abi.encode("")
        );
        vm.mockCall(
            address(_vault),
            abi.encodeWithSelector(_vault.availableYieldBalance.selector),
            abi.encode(balance*2)
        );

        uint firstTime = block.timestamp + 4 weeks;
        vm.warp(firstTime);
        vm.expectRevert(abi.encodeWithSelector(PrizeVault.LiquidationTokenOutNotSupported.selector, tokenOut));
        liquidationPair.swapExactAmountOut(address(receiver), 1e18, 100e18, "");
    }

    function test_TpdaLiquidationPair_ErrorOf_tokenIn() public {
        uint64 targetAuctionPeriod = 1 hours;
        uint192 auctionTargetPrice = 1e18;
        uint256 smoothing = 0.1e18;

        TpdaLiquidationPairFactory tpdafactory = new TpdaLiquidationPairFactory();
        PrizeVault _vault;

        address tokenIn = makeAddr("tokenIn");
        address receiver = makeAddr("receiver");

        asset.mint(address(this), yieldBuffer);
        asset.approve(address(vaultFactory), yieldBuffer);

        _vault = vaultFactory.deployVault(
            name,
            symbol,
            yieldVault,
            PrizePool(address(prizePool)),
            claimer,
            address(this),
            yieldFeePercentage,
            yieldBuffer,
            owner
        );
        asset.mint(address(_vault), 1234e18 * 2);

        TpdaLiquidationPair liquidationPair = tpdafactory.createPair(
            _vault,
            tokenIn,
            address(asset),
            targetAuctionPeriod,
            auctionTargetPrice,
            smoothing
        );

        vm.prank(owner);
        _vault.setLiquidationPair(address(liquidationPair));

        uint256 balance = 1234e18;
        vm.mockCall(
            address(_vault),
            abi.encodeWithSelector(_vault.liquidatableBalanceOf.selector, address(asset)),
            abi.encode(balance)
        );
        vm.mockCall(
            address(_vault),
            abi.encodeWithSelector(
                _vault.transferTokensOut.selector,
                address(liquidationPair),
                receiver,
                address(asset),
                balance
            ),
            abi.encode("")
        );
        vm.mockCall(
            address(_vault),
            abi.encodeWithSelector(_vault.availableYieldBalance.selector),
            abi.encode(balance*2)
        );

        uint firstTime = block.timestamp + 4 weeks;
        vm.warp(firstTime);
        vm.expectRevert(abi.encodeWithSelector(PrizeVault.LiquidationTokenInNotPrizeToken.selector, tokenIn, prizePool.prizeToken()));
        liquidationPair.swapExactAmountOut(address(receiver), 1e18, 100e18, "");
    }

Code Snippet

https://github.com/sherlock-audit/2024-05-pooltogether/blob/1aa1b8c028b659585e4c7a6b9b652fb075f86db3/pt-v5-tpda-liquidator/src/TpdaLiquidationPairFactory.sol#L48-L63 https://github.com/sherlock-audit/2024-05-pooltogether/blob/1aa1b8c028b659585e4c7a6b9b652fb075f86db3/pt-v5-tpda-liquidator/src/TpdaLiquidationPair.sol#L162 https://github.com/sherlock-audit/2024-05-pooltogether/blob/1aa1b8c028b659585e4c7a6b9b652fb075f86db3/pt-v5-vault/src/PrizeVault.sol#L767

Tool used

Manual Review

Recommendation

 function createPair(
        ILiquidationSource _source,
        address _tokenIn,
        address _tokenOut,
        uint64 _targetAuctionPeriod,
        uint192 _targetAuctionPrice,
        uint256 _smoothingFactor
    ) external returns (TpdaLiquidationPair) {
+        require(_tokenIn == address(prizePool.prizeToken(), "tokenIn Sus");
+         require(_tokenOut == _source || _tokenOut == _source.asset, "tokenOut sus" );
        TpdaLiquidationPair _liquidationPair = new TpdaLiquidationPair(
            _source,
            _tokenIn,
            _tokenOut,
            _targetAuctionPeriod,
            _targetAuctionPrice,
            _smoothingFactor
        );

[sherlock-admin2]

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

infect3d commented:

not an issue as the prizeVault owner verify the liquidation pair before setting it

[nevillehuang]

Invalid, sponsor comments:

• agree invalid since bots are supposed to verify the tokenOut and tokenIn
• these contracts are multipurpose and should not be restricted with the prize pool or vault interface