previewRate() in Router.sol will always revert if the first command is a transfer

[hats-bug-reporter]

Github username: --
Twitter username: --
Submission hash (on-chain): 0x62fb4103a30989fdc9fe46119a83c44345dba191f468db8b5291a3c091ac52b3
Severity: high

Description:
Description
The previewRate() function in Router.sol is meant to simulate the execute() operation and returns the expected preview rate value. It uses a TokenBalance[] array to track token balance changes during the preview operations.

A critical issue exists where the function will always revert if the first command is a transfer operation (TRANSFER command) that attempts to decrease a token balance. This occurs because:

1. The TokenBalance[] array is initially empty
2. When processing a transfer command, _decreasePreviewTokenValue() is called
3. _decreasePreviewTokenValue() attempts to find the token in the empty balance array
4. Since the token isn't found (as no balances have been added yet), it reverts with BalanceUnderflow

This prevents any command sequence starting with a transfer from being previewed, which is a common legitimate use case.

Attack Scenario\

1. User wants to preview a complex swap that starts with transferring tokens from the router contract to their own address/ some other address.
2. The command sequence starts with TRANSFER followed by other operations
3. When calling previewRate(), the function will always revert on the first command
4. This prevents users from getting rate previews for many legitimate transaction sequences

Attachments

1. Proof of Concept (PoC) File

    function testPreviewArbitraryTransferShouldNotRevert() public {
        bytes memory commands = abi.encodePacked(
            bytes1(uint8(Commands.TRANSFER)),
            bytes1(uint8(Commands.TRANSFER_FROM))
        );
        bytes[] memory inputs = new bytes[](2);
        inputs[0] = abi.encode(principalToken, address(this), 1e18);
        inputs[1] = abi.encode(principalToken, 1e18);
        uint256 previewRate = router.previewRate(commands, inputs);
    }

2. Revised Code File (Optional)
   Fix is non-trivial. There needs to be a way to track negative balances to ensure that final balances are settled.

[yanisepfl]

Hello,

We classified this issue as Invalid because:

• It cannot result in direct theft of funds.
• Our router is not designed to be used with TRANSFER as the first command.

Thanks