Sparkn

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

Deployed proxies not disposed after usage

Summary

Deployed contest contracts not explicitly disposed off after usage

Vulnerability Details

Project README states the following
Proxy contracts are supposed to be disposed after the contest is over.
However after contest usage and deploys to distribute; the proxy contracts at their deployed addresses are still in existence without any restrictions that ProxyFactory.sol functions cant still call them; additionally funds can still be sent to the addresses

It is possible to call the various deploy...() functions in ProxyFactory.sol many times as they will not revert since redoing create2 will result in zero address return for Proxy which succeeds in low level calls that don't check for zero address

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));
return proxy;
}
// when above function is called many times will not revert but after first time with same parameters and salt will return address(0) for proxy
function _distribute(address proxy, bytes calldata data) internal {
(bool success,) = proxy.call(data);
if (!success) revert ProxyFactory__DelegateCallFailed();
emit Distributed(proxy, data);
}
// function above will succeed since address(0).call(data) will return success when doing low level call to zero address

Impact

  1. This is not consistent with the interpretation of documentation

  2. Sponsors may continue to send funds to these contest contracts thinking still active

  3. Other users or random people may continue to send funds these contract by error or deliberately implying admin/owner needs to rescue these funds using gas

  4. Way after these contracts not being used; ProxyFactory.sol Owner may be able to use rescue funds functionality to redeem tokens sent by error to these addresses redeeming to own address for own benefit

Tools Used

Manual Analysis

Recommendations

It may be recommended to favor explicitness over implicitness and make it clear in code that the proxies after life cycle are disposed and not fit for use by using any combination of the following options

  1. Ensure address proxy != address(0) in the _deploy() function or in many of the deploy functions in the code

  2. Safe way to selfdestruct them(given dangers of delegatecall) so using proper access control and other security mechanisms to destroy the contracts with selfdestruct safely so that there truly is no contract code there after usage

  3. When contest lifecycle finishes have a mapping changed to false of all addresses that have deployed and distributed so these can be queried e.g

mapping(address => bool) ended;
function deploy....(...) {
...existing logic;
_distribute(proxy, data);
ended[proxy] = true; // update various deploy functions to add this
return proxy
}
function isEnded(address _proxy) .. {
return ended[_proxy]; //so stakeholders before interacting with contest can query if it is disposed
}
  1. Or alternatively ensure the various deployAnd... functions in ProxyFactory.sol can only be called once using some sort of a flag

Support

FAQs

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