Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Submission Details
Severity: low
Valid

External calls in an un-bounded `for-loop` may result in a DOS

Summary

Vulnerability Details

if winners.length is too big (>1900), it will revert an error "EvmError: OutOfGas" due to block gas limit

PoC Code:

// test/integration/ProxyFactoryTest.t.sol
function createDataManyUsers() public view returns (bytes memory data) {
uint256 addrCount = 95 * 10 * 2;
address[] memory winners = new address[](addrCount);
uint256[] memory percentages_ = new uint256[](addrCount);
for (uint256 i; i < addrCount; ) {
winners[i] = address(uint160(i + 1));
percentages_[i] = 9500 / addrCount;
unchecked {
++i;
}
}
data = abi.encodeWithSelector(
Distributor.distribute.selector,
jpycv2Address,
winners,
percentages_,
""
);
}
function testManyUser()
public
setUpContestForJasonAndSentJpycv2Token(organizer)
{
bytes32 randomId_ = keccak256(abi.encode("Jason", "001"));
bytes memory data = createDataManyUsers();
vm.warp(16 days);
vm.startPrank(factoryAdmin);
proxyFactory.deployProxyAndDistributeByOwner{gas: 30_000_000}(
organizer,
randomId_,
address(distributor),
data
);
}

run command:

forge test -vvv --match-test testManyUser

results:

│ │ │ │ ├─ emit Transfer(from: 0x717a630f36795235Aa9Bd5bf6DF42c431d2875FD, to: 0x0000000000000000000000000000000000000413, value: 5000000000000000000 [5e18])
│ │ │ │ └─ ← true
│ │ │ ├─ [3068] MockERC20::transfer(0x0000000000000000000000000000000000000414, 5000000000000000000 [5e18])
│ │ │ │ └─ ← "EvmError: OutOfGas"
│ │ │ └─ ← "EvmError: OutOfGas"
│ │ └─ ← "EvmError: Revert"
│ └─ ← "ProxyFactory__DelegateCallFailed()"
└─ ← "ProxyFactory__DelegateCallFailed()"
Test result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 20.12ms
Ran 1 test suites: 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/integration/ProxyFactoryTest.t.sol:ProxyFactoryTest
[FAIL. Reason: ProxyFactory__DelegateCallFailed()] testManyUser() (gas: 30691967)
Encountered a total of 1 failing tests, 0 tests succeeded

Impact

All token will be locked in contract.

Tools Used

Foundry

Recommendations

  1. Add an upper limit condition for winners.length at the beginning of function distribute

  2. Redesign the implementation of the distribute function, if the winners.length > maxLengthLimit, use batch distribution.

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.

Give us feedback!