Cross-chain liquidity swaps can be executed with more vault tokens than the vault's token balance.

[hats-bug-reporter[bot]]

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

Description: Description\ When initiating a cross chain liquidity swap in function sendLiquidity, there are no validations in place to verify that the amount of vault tokens about to be exchanged isn't more than the vault's token balance before sending the liquidity across chains. https://github.com/catalystdao/catalyst/blob/27b4d0a2bca177aff00def8cd745623bfbf7cb6b/evm/src/CatalystVaultVolatile.sol#L890C1-L896C11

This allows a situation where malicious cross chain liquidity swaps are made with more vault tokens than the actual vault's token balance.

Attack Scenario\ The vulnerability can be exploited to exchange more vault tokens than the vault's token balance during cross chain liquidity swaps.

Proof of Concept (PoC) File

Add this test to sendLiquidity.t.sol and run forge test --mt test_Malicious_sendLiquidity

    function test_Malicious_sendLiquidity(bytes32 channelId, uint32 swapSizePercentage, address toAccount) external virtual {
        vm.assume(toAccount != address(0));
        vm.assume(swapSizePercentage != type(uint32).max);
        address[] memory vaults = getTestConfig();
        require(vaults.length >= 2, "Not enough vaults defined");

        uint8 fromVaultIndex = 0;
        uint8 toVaultIndex = 1;

        address fromVault = vaults[fromVaultIndex];
        address toVault = vaults[toVaultIndex];

        setConnection(fromVault, toVault, channelId, channelId);

        uint256 amount = Token(fromVault).balanceOf(address(this)) * swapSizePercentage / (2**32 - 1);

        ICatalystV1Structs.RouteDescription memory routeDescription = ICatalystV1Structs.RouteDescription({
            chainIdentifier: channelId,
            toVault: convertEVMTo65(toVault),
            toAccount: convertEVMTo65(toAccount),
            incentive: _INCENTIVE
        });

        ICatalystV1Vault(fromVault).sendLiquidity{value: _getTotalIncentive(_INCENTIVE)}(
            routeDescription,
            amount + 10000, // more tokens than it's balance
            [uint256(0), uint256(0)],
            address(this),
            hex""
        );
    }

PoC file attached below.

Potential fix

Ensure that the amount of tokens to be exchanged isn't more than the vault's token balance.

Files:

• SendLiquidity.t.sol (https://hats-backend-prod.herokuapp.com/v1/files/QmaVwV71poaytibXGx3Kt3EbyFTgLgPaVcjV3r1md6x1VM)

[reednaa]

The PoC using fuzzing, I think that is why it seems possible.

The check for more than balance is here: https://github.com/hats-finance/Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4/blob/fba322fab023a9206183fb455e9f86facd550d8a/evm/src/CatalystVaultVolatile.sol#L878

[0xisaacc]

The PoC using fuzzing, I think that is why it seems possible.

Yes. At some point, it will be possible to send more vault tokens than the vault's token's balance

[reednaa]

Please implement PoC without fuzzing or show me relevant code.

[0xisaacc]

Let's recap.

• There is a serious bug in the smart contract. More vault tokens can be swapped than the actual token balance of the vault.

• The bug is a rare edge case that probably wouldn't have been caught through normal testing.

• Your point is on how the bug was found, rather than the bug itself.

[reednaa]

Your fuzzing test contains:

uint256 amount = Token(fromVault).balanceOf(address(this)) * swapSizePercentage / (2**32 - 1);

So the user is never withdrawing their full amount except when swapSizePercentage == type(uint32).max minus the 10000.

Could you explain why the tests fails when swapSizePercentage is high enough that more than the user's balance is withdrawn?

And as a Security researcher, would you be able to provide me with the failing cases of your test so we can examine edges of the test and figure out where the fail area for the input swapSizePercentage is?

[0xisaacc]

show me relevant code.

There should have been a check like this in function sendLiquidity before executing the cross chain swap.

require(vaultTokens <= vaultToken.balanceOf(vault)); 

This will prevent the issue.

[reednaa]

This is the check you want implemented?

https://github.com/hats-finance/Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4/blob/fba322fab023a9206183fb455e9f86facd550d8a/evm/src/CatalystVaultVolatile.sol#L878

https://github.com/transmissions11/solmate/blob/e8f96f25d48fe702117ce76c79228ca4f20206cb/src/tokens/ERC20.sol#L196