code-423n4 / 2022-06-yieldy-findings

0 stars 0 forks source link

Possible DOS (out-of-gas) on loops. #94

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

Lines of code

https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/BatchRequests.sol#L16 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/BatchRequests.sol#L36 https://github.com/code-423n4/2022-06-yieldy/blob/main/src/contracts/BatchRequests.sol#L91

Vulnerability details

Impact

There is possible to get a out of gas issue while iterating the for loop. Please take a look to this link; https://github.com/wissalHaji/solidity-coding-advices/blob/master/best-practices/be-careful-with-loops.md

Proof of Concept

Lets say i want to run the function on BatchRequests.sol#L14 and i got a lot of contracts pending for withdrawal.

Tools Used

Manual revision

Recommended Mitigation Steps

Use this pattern;

    /**
        @notice sendWithdrawalRequests on all addresses in contracts
     */
    function sendWithdrawalRequests(uint256 from, uint256 to) external {
        uint256 contractsLength = contracts.length;
        require(from < contractsLength, "Invalid from");
        require(to <= contractsLength, "Invalid to");
        for (uint256 i = from; i < to; ) {
            if (
                contracts[i] != address(0) &&
                IStaking(contracts[i]).canBatchTransactions()
            ) {
                IStaking(contracts[i]).sendWithdrawalRequests();
            }
            unchecked {
                ++i;
            }
        }
    }
toshiSat commented 2 years ago

duplicate #102