Unbounded loops in several contracts

[code423n4]

Handle

shw

Vulnerability details

Impact

Unbounded loops, for example, iterating over a variable-length array, could cost an arbitrarily amount of gas. If the required gas of executing the loop exceeds the block gas limit, the transactions will revert, causing some critical functions to be disabled.

Proof of Concept

For example, the claimAllForMember function of Dao iterates the listedBondAssets to claim all unlocked bonded LP tokens of a member. However, the listedBondAssets array contains the historical array of all past bond listed assets, whose length continuously increases over time. Thus, if the array length is too large, this function could cost gas more than the block gas limit. Even not, users calling this function could suffer from large gas consumption. See the following links for all unbounded loops.

Referenced code: poolFactory.sol#L100-L104 synthVault.sol#L122-L128 Dao.sol#L278-L283

Recommended Mitigation Steps

Consider allowing users to decide the start and end indices when iterating the loop to avoid iterating the whole array. For example, re-write the claimAllForMember as follows:

function claimAllForMember(address member, uint start, uint end) external returns (bool) {
    for (uint i = start; i < end; i++) {
        uint claimA = calcClaimBondedLP(member, listedBondAssets[i]); // Check user's unlocked Bonded LPs for each asset
        if (claimA > 0) {
           _BONDVAULT.claimForMember(listedBondAssets[i], member); // Claim LPs if any unlocked
        }
    }
    return true;
}

[SamusElderg]

Duplicate of #37

[ghoul-sol]

Duplicate of #37 so low risk