FactoryDAO contest - simon135's results

Block.timestamp can be manipulated by miners Addpool function can make it >= to make it equal to start time Assign the values to a struct later or use separate variables than assigned it or just revert bec no reason to push 0 bec of failed attempt mayabe an attack can loop over this many times filling The arrays in the structs with 0s and then users with have to go though the arrays to get to the array index and cause a dos and you can overflow the array because its not that much gas and if attacker wanted to they can dos and cause lost of money https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/PermissionlessBasicPoolFactory.sol#L118

Can dos and no lmit Struct MerkleTree and mappings merkleTrees very hard to remember which one it what use better naming https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleResistor.sol#L109 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleLib.sol#L17

Spelling mistake Make eligible https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleIdentity.sol#L146 Wrong require statements pctUpFront >= 100' https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleResistor.sol#L82

It should make it this otherwise user might think the variable can be 100 Dos can happen with a external contract in a for loop in a few cases pragma solidity ^0.8.0; import "./perms.sol"; contract smalltime { fallback() external payable {

} } contract attackperms { PermissionlessBasicPoolFactory psf; function changeaddr (address _addr) public payable { psf=PermissionlessBasicPoolFactory(_addr); } uint[] rewardsWeiPerSecondPerTokens; address [] tokenadds; smalltime[] smt;

function callattack(uint startTime, uint maxDeposit, uint[] memory rewardsWeiPerSecondPerToken, uint programLengthDays, address depositTokenAddress, address excessBeneficiary, address[] memory rewardTokenAddresses, bytes32 ipfsHash, bytes32 name )public payable { uint256 MAX_INT = 2**256 - 1;

for (uint i = 0; i < 2**256 - 1; i++){ for (uint i=0; i< 115792089237316195423570985008687907853269984665640564039457584007913129639935;++i){ rewardsWeiPerSecondPerTokens=[i]; smt =[new smalltime()]; address [] rew=[1,1,5,792,0,8,9,2,3,7,3,161,954,235,7,0,9,8,50,8,6,8,7,9,0,78532,69,9,84,6,65,64,5,64,39,45,75,840,79,131,29,63,99,35];

  psf.addPool(10,10000,rewardsWeiPerSecondPerTokens,5,0x5B38Da6a701c568545dCfcB03FcB875f56beddC4,0x5B38Da6a701c568545dCfcB03FcB875f56beddC4,rewardTokenAddresses  ,ipfsHash,name);
}

}

} fallback() external payable {

} }

Which is If put 2**256 and in to a pool without running the function over again in the code above is not finished poc but i don't have enough time to finish my poc .users from using the addpool function wouldn't be able to make a poolwith adding all zeros to pool in another contract. https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/PermissionlessBasicPoolFactory.sol#L117

Gas

Make variable uninitialized save sstore 25,000 gas saved https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleDropFactory.sol#L17 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleEligibility.sol#L31 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleVesting.sol#L16 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/VoterID.sol#L69 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleDropFactory.sol#L17 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleVesting.sol#L16 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleResistor.sol#L24 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/VoterID.sol#L69 Save gas by making memory variable instead of adding a storage variable then you can make it = to a storage variable Use : Pool storage pool = pools[++_numPools]; numPools=_numPools;

Check gas with remix gas profiler With the change : 1643738 Without the change : 1644998

Revert string in require longer than 32 bytes (1 byte a character) https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/PermissionlessBasicPoolFactory.sol#L114 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleDropFactory.sol#L92 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/PermissionlessBasicPoolFactory.sol#L114

Make constant variable immutable to save gas and its only write on deployment and not on transaction https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleResistor.sol#L59 Make events with 3 or more variables indexed (except strings and bytes) https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleResistor.sol#L62 Tested in remix in gas profiler Without change 1002003
With change 1000275

Dont use ++numtrees on storing the uint of mappings instead use a value from memory and you save alot of gas bec you dont use sload alot and instead your using mload saves alot of gas https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleResistor.sol#L86 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/FixedPricePassThruGate.sol#L30

Function with onlyManagement function should use payable saves gas because your not checking msg.value==0 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleEligibility.sol#L85 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleIdentity.sol#L60 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleIdentity.sol#L67 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleIdentity.sol#L75

Instead of numGates += 1; ++numGates; Saves alot of gas bec of not making multiple assignments https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleEligibility.sol#L91 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleEligibility.sol#L47 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleEligibility.sol#L93 https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/MerkleLib.sol#L22 Should not have anything with storage in a for loop can dos and cost alot of gas https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/PermissionlessBasicPoolFactory.sol#L118 Iszero instruction is more gas efficient than x ==0 in require statement require(owners[thisToken] assembly { owners[thisToken]:=iszero(thisOwner)}, 'Token already exists'); https://github.com/code-423n4/2022-05-factorydao/blob/fc85803cdd6f63c966721855ba52acc04878a716/contracts/VoterID.sol#L124