trauki - Medium - `fillThreshold` can be 0

[sherlock-admin]

trauki

medium

Medium - fillThreshold can be 0

Summary

When swappers submit their order, they can specify a fillThreshold value which is the minimum amount of input tokens they will accept for the order to be filled. This value is never checked meaning swappers can submit orders with 0 as fillThreshold.

Vulnerability Detail

Fillers can fill these types of orders with a trival amount of input tokens.

Impact

Swappers who input fillThreshold as 0 can have their orders filled with a minuscule amount of tokens, forcing the swapper to submit a new order to swap anything, which can result in loss of the benefit from making a trade at a certain price/time.

In this modified test, a swapper has an order to swap 100e18 tokens of tokenIn for 200e18 tokens of tokenOut. Since the fillThreshold was never correctly given a value, our filler partially fills the order with a single WEI of tokenIn. This swap successfully executes and the swapper trades 1 WEI of tokenIn for 2 WEI of tokenOut.

Obviously, this amount is frivolous and the order might as well have been canceled, but since it was successfully executed, the swapper could be under the impression that their order was filled correctly and they could believe they hold a position in the tokenOut (until they check their actual balance).

Example of swapper being negatively affected like this:

1. Alice anticipates price movement and announces a swap. She could even use some off-chain logic to listen for her order to be filled and check that her new balance is greater than her previous (or that her new tokenIn balance is less than her original balance).
2. Bob fills her order with a trivial amount of tokens.
3. Alice's off-chain logic is executed and she now knows that her order was filled and that her tokenOut balance has increased.
4. Price movement occurs.
5. Alice tries to swap out of the tokenOut only to realize her position in this asset is insignificant.

This could mean that Alice lost money by her original position losing value, or lost out on a profitable trade if the token she wanted to swap for increased in value.

Even if Alice realized her mistake and sent in a new order with a reasonable fillThreshold or canceled her order, the original order could still be executed.

Code Snippet

function test_ExecuteTrivialPartialFill() public {
        GladiusOrder memory order = defaultAsk();
        (
            InputToken memory inPf,
            OutputToken[] memory outPf
        ) = applyDecayAndPartition(order, quantity);

        mintAndApprove(
            address(tokenIn),
            order.input.endAmount,
            address(swapper)
        );
        tokenOut.mint(address(fillContract), outPf[0].amount);
        tokenOut.forceApprove(
            address(fillContract),
            address(reactor),
            outPf[0].amount
        );

        (
            uint256 swapperBalanceIN_0,
            uint256 swapperBalanceOUT_0
        ) = saveBalances(address(swapper));
        (uint256 fillerBalanceIN_0, uint256 fillerBalanceOUT_0) = saveBalances(
            address(fillContract)
        );

        console2.log("TokenIn balances prior to fill:");
        console2.log(tokenIn.balanceOf(swapper)); // Starts with 100e18 tokenIn
        console2.log(tokenIn.balanceOf(address(fillContract))); // Starts with 0 tokenIn
        console2.log("TokenOut balances prior to fill:");
        console2.log(tokenOut.balanceOf(swapper)); // Starts with 0 tokenOut
        console2.log(tokenOut.balanceOf(address(fillContract))); // Starts with 2 tokenOut 

        fillContract.execute(generateSignedOrder(order), quantity); // Execute the partial fill with quantity set as 1 WEI

        console2.log("TokenIn balances post fill:");
        console2.log(tokenIn.balanceOf(swapper)); // Ends with 100e18 - 1 WEI tokens after fill
        console2.log(tokenIn.balanceOf(address(fillContract))); // Ends with 1 tokenIn after fill

        console2.log("TokenOut balances post fill:");
        console2.log(tokenOut.balanceOf(swapper)); // Ends with 2 WEI of tokenOut
        console2.log(tokenOut.balanceOf(address(fillContract))); // Ends with 0 tokenOut

        //-------------------- SWAPPER ASSERTIONS

        /// @dev 'swapper' spent only 1 WEI input tokens.
        assertEq(
            (swapperBalanceIN_0 - tokenIn.balanceOf(swapper)),
            inPf.amount
        );
        /// @dev 'swapper' bought only 2 WEI output tokens.
        assertEq(
            (tokenOut.balanceOf(swapper) - swapperBalanceOUT_0),
            outPf[0].amount
        );

        // //-------------------- FILLER ASSERTIONS

        // /// @dev 'filler' bought 1 WEI input tokens
        assertEq(
            (tokenIn.balanceOf(address(fillContract)) - fillerBalanceIN_0),
            inPf.amount
        );
        /// @dev 'filler' spent 2 WEI output tokens
        assertEq(
            (fillerBalanceOUT_0 - tokenOut.balanceOf(address(fillContract))),
            outPf[0].amount
        );

    }

Tool used

Manual Review

Recommendation

Add a conditional check to ensure the fillThreshold is non-zero:

function _validateThreshold(
        uint256 _fillThreshold,
        uint256 _inAmt
    ) internal pure {
        if (_fillThreshold > _inAmt || _fillThreshold == 0) revert InvalidThreshold();
    }

or that it is greater than a certain minimum amount such as 1% of the original order amount:

function _validateThreshold(
        uint256 _fillThreshold,
        uint256 _inAmt
    ) internal pure {
        if (_fillThreshold > _inAmt || _fillThreshold < _inAmt.divWadUp(100)) revert InvalidThreshold();
    }

[sherlock-admin]

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

tsvetanovv commented:

Ivalid. This is a user mistake and is invalid according to Sherlock rules

PNS commented:

User input validation: User input validation to prevent user mistakes is not considered a valid issue.

0xAadi commented:

Invalid: The user should accept the effect of using fillThreshold as zero, the protocol allow fillThreshold as zero