Sparkn

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

Wrong funds from mismatched proxy can be distributed mistakenly by the owner

Summary

In distributeByOwner() function, owner has to provide the proxy address as the params to call for distribution. However, there is only a validation of zero address for proxy param without checking if it really matches the proxy address of that contest calculated by salt.

Vulnerability Details

Owner may mistakenly provide wrong proxy address, which leads to funds losing and bypassing contest distribution expiration checks. There are 2 types of proxy that are affected by this.

Case 1:

  • Affecting the proxy which was deployed but somehow revert on distribution stage (Organizer passing wrong data params).

Case 2:

  • Affecting the proxy where funds have been distributed but the sponsor mistakenly sends fund to it again.

POC

Paste this function and modifier into ProxyFactoryTest.t.sol:

https://gist.github.com/sonny2k/cd0126ff0adf658bd744b215cfe3a9e2

To run this, type forge test --mt testSucceedsNonExistenceProxyDistributeByOwner -vvvvv

In this POC I stimulate case number 1 above, Owner wrongly puts the proxy address of Sonny while the purpose is to rescue funds from Jason contest. And Jason contest's proxy was deployed but failed at the distribution stage with false params. Resulting in 10000 ether transferred to Stadium Address even though Sonny contest isn't expired (28 days), and the proxy hasn't been deployed.

Impact

Breaking contest distribution expiration mechanism and fund losing. Since this issue requires the proxy to be deployed already to take effect, I moved this to medium.

Tools Used

Manual Analysis

Recommendations

Add a validation that the provided proxy param has to equal the proxy address calculated based on the salt of the purposed organizer, contestId, implementation.

...
+ error ProxyFactory_MismatchedProxyAddress();
...
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);
+ address calculatedProxy = getProxyAddress(salt, implementation);
+ if (proxy != calculatedProxy) revert ProxyFactory_MismatchedProxyAddress();
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);
}

Support

FAQs

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