Sparkn

CodeFox Inc.
DeFiFoundryProxy
15,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Owner of ProxyFactory can arbitrarily call any function on any address, including circumventing the `EXPIRATION_TIME` for a contest

toshii

Summary

As per the natspec comments for the distributeByOwner function, the owner should only be able to call functions on the Proxy when block.timestamp >= saltToCloseTime[salt] + EXPIRATION_TIME, however this check is never actually enforced. Instead, there is a proxy address input, which allows the owner to arbitrarily call any function on any address.

Vulnerability Details

Consider the implementation for distributeByOwner:

function distributeByOwner(
address proxy,
address organizer,
bytes32 contestId,
address implementation,
bytes calldata data
) public onlyOwner {
if (proxy == address(0)) revert ProxyFactory__ProxyAddressCannotBeZero();
bytes32 salt = _calculateSalt(organizer, contestId, implementation);
if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
// distribute only when it exists and expired
if (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) revert ProxyFactory__ContestIsNotExpired();
_distribute(proxy, data);
}

Although they check the saltToCloseTime[salt] for the salt created from the inputted organizer, contestId, and implementation, this has no relation to the proxy address which the owner is actually calling. For example, this means they can circumvent the EXPIRATION_TIME check.

Impact

Owner can arbitrarily call functions for any contract, not just contests, and can also circumvent time checks which are meant to be enforced.

Tools Used

Manual review

Recommendations

Rather than have proxy as an input to this function, proxy should be calculated using the getProxyAddress function.

Support

FAQs

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

Give us feedback!