Sparkn

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

The `distributeByOwner` lock period can be bypassed

klaus

Summary

Even if the lock period does not pass, the owner can bypass the lock period and call distributeByOwner.

Vulnerability Details

Since it is not checked whether the proxy parameter is a proxy address using this salt, by using organizer, contestId, implementation of contest which already passed lock period, owner can bypass lock period.

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

https://github.com/Cyfrin/2023-08-sparkn/blob/47c22b818818af4ea7388118dd83fa308ad67b83/src/ProxyFactory.sol#L205-L218

Impact

The owner can bypass the lock period and distribute the prize

Tools Used

vscode

Recommendations

Check if proxy is actually made by the salt

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();
+ require(getProxyAddress(salt, implementation) == proxy);
_distribute(proxy, data);
}

Support

FAQs

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