Sparkn

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

Their is no check that the `salt==contest` in `distributeByOwner`

ohi0b

Summary

Even though this is an owner function there should be a check to make sure the owner calls distribute on the right contest.

Vulnerability Details

When the owner calls distritbuteByOwner the proxy is diffrent then the salt causing a different contest to get distributed.
In this code snippet their is no verification that the salt=proxy

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);
}

Impact

The impact is a break that the contest has to be finished for it to be distributed.
Even though this is an Owner function the right constraints should be applied.Plus this is not a centralization issue,it's more of a check for a best practice.That if not followed can end in really bad state.

Tools Used

Recommendations

) public onlyOwner {
if (proxy == address(0)) revert ProxyFactory__ProxyAddressCannotBeZero();
bytes32 salt = _calculateSalt(organizer, contestId, implementation);
+ if ( proxy != getProxyAddress(salt,implementation) revert ProxyFactory__DifferentProxy();
if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();

Support

FAQs

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

Give us feedback!