Sparkn

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

Zero Address Proxies

Summary

Returned proxies can be zero addresses
Additionally low level calls are not checked for contract existence or zero address

Vulnerability Details

Since many of the deploy functions can be called many times as create2 does not fail when called again with same salt but returns address(0) after first call as there is already a contract there

function deployProxyAndDistribute(bytes32 contestId, address implementation, bytes calldata data)
// example deploy functions use code below but don't check if proxy == address(0)
address proxy = _deployProxy(organizer, contestId, implementation);
_distribute(proxy, data);
// _distribute uses low level call does not check for contract existence so will always bring back success

Consider the following process flow (maybe its a front end that doesn't check values but assumes values from function calls are correct and chain feeds them into the next step etc)

  1. Owner setsContest => setContest(0xOrganizer, 0xContestId01, closeTime0001, 0xImplementation) -> salt result is 0xSalt01

  2. Process flow calls => getProxyAddress(0xSalt01, 0xImplementation) -> proxy result is 0xProxy01

  3. Process flow informs Sponsors users to use 0xProxy01 to send funds, for competition etc

  4. Sponsor01 sends WhitelistedTokensA to 0xProxy01 and NotWhielistedTokensB to 0xProxy01

  5. Competitions finishes and owner or organizer informed need to distribute funds plus rescue NotWhielistedTokensB(maybe this is built into front end in some chain)

  6. Owner calls function deployProxyAndDistributeByOwner(0xProxy01, 0xOrganizer, 0xContestId01, 0xImplementation, data) which is based on 0xSalt01 and returns correctly 0xProxy01 address stored in front end chain or process flow
    [At this stage idea is to use this return value to feed into function distributeByOwner(
    address proxy, // inject here to rescue NotWhielistedTokensB
    address organizer,
    bytes32 contestId,
    address implementation,
    bytes calldata data
    )]

  7. However since nothing restricts deployProxyAndDistributeByOwner(...) to be called again, owner calls this by error e.g click button again on front end or reinitatied process flow ...
    deployProxyAndDistributeByOwner(0xProxy01, 0xOrganizer, 0xContestId01, 0xImplementation, data) since same salt
    address proxy = address(new Proxy{salt: salt}(implementation)); will return address(0)
    therefore this repeated call now feeds to front end or process flow address(0) to saving funds

  8. Owner now tries to save funds by calling function distributeByOwner(
    address(0),
    0xOrganizer,
    0xContestId01,
    0xImplementation,
    data [pass in token address for NotWhielistedTokensB ]
    )
    Since _distribute(proxy, data) involves low level call address(0).call(data) it will return success and function distributeByOwner() will succeed and owner will relax that NotWhielistedTokensB where rescued when nothing was done

Impact

Results in various deploy and distribute functions being able to be called many times with same values; wheres its only useful the first time as any subsequent calls are operating on address proxy = address(0) returned from calling create2 again with same salt;

Such address(0) can lead to problems in the protocol/project as illustrated in the example Process Flow Above

Tools Used

Manual Analysis

Recommendations

Recommended to check that return address from _deployProxy is not address(0) e.g consider ways like below

function _deployProxy(address organizer, bytes32 contestId, address implementation) internal returns (address) {
bytes32 salt = _calculateSalt(organizer, contestId, implementation);
address proxy = address(new Proxy{salt: salt}(implementation));
if (proxy == address(0)) revert ProxyFactory__NoZeroAddress(); // add zero address checks
return proxy;
}

Support

FAQs

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