hats-finance / Catalyst-Exchange-0x3026c1ea29bf1280f99b41934b2cb65d053c9db4

Other
1 stars 2 forks source link

For loops in public or external functions should be avoided due to high gas costs and possible DOS #15

Open hats-bug-reporter[bot] opened 8 months ago

hats-bug-reporter[bot] commented 8 months ago

Github username: @saidqayoumsadat Twitter username: saqsadat143 Submission hash (on-chain): 0x0e9b46da44b881bc0041ddc79eb1fcdf9f371d37ad7719099de2a2136d413c5b Severity: low

Description: Description\ In Solidity, for loops can potentially cause Denial of Service (DoS) attacks if not handled carefully. DoS attacks can occur when an attacker intentionally exploits the gas cost of a function, causing it to run out of gas or making it too expensive for other users to call. Below are some scenarios where for loops can lead to DoS attacks: Nested for loops can become exceptionally gas expensive and should be used sparingly

  1. Proof of Concept (PoC) File
file: /src/CatalystFactory.sol

65   function deployVault(
        address vaultTemplate,
        address[] calldata assets,
        uint256[] calldata init_balances,
        uint256[] calldata weights,
        uint256 amp,
        uint256 vaultFee,
        string memory name,
        string memory symbol,
        address chainInterface
    ) override external returns (address) {
        // Check if an invalid asset count has been provided
        require(assets.length != 0);  // dev: invalid asset count
        // Check if an invalid weight count has been provided
        require(weights.length == assets.length); //dev: invalid weight count
        // init_balances length not checked: if shorter than assets, the funds transfer loop
        // will fail. If longer, values will just be ignored.

        // Create a minimal transparent proxy:
        address vault = Clones.clone(vaultTemplate);

        // The vault expects the balances to exist in the vault when setup is called.
        for (uint256 it; it < assets.length;)

https://github.com/catalystdao/catalyst/blob/27b4d0a2bca177aff00def8cd745623bfbf7cb6b/evm/src/CatalystFactory.sol#L65C4-L87C48

file: src/CatalystVaultAmplified.sol

170    function updateMaxUnitCapacity() external {
        uint256 maxUnitCapacity;
        for (uint256 it; it < MAX_ASSETS;) {

https://github.com/catalystdao/catalyst/blob/27b4d0a2bca177aff00def8cd745623bfbf7cb6b/evm/src/CatalystVaultAmplified.sol#L170C1-L172C45

file: /src/CatalystVaultVolatile.sol

199                for (uint256 it; it < MAX_ASSETS;) {

https://github.com/catalystdao/catalyst/blob/27b4d0a2bca177aff00def8cd745623bfbf7cb6b/evm/src/CatalystVaultVolatile.sol#L199C1-L199C53

reednaa commented 8 months ago

Non-issue. How would you otherwise handle it?

Propose a solution for the gas competition.