Sparkn

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

Can call arbitrary function of arbitrary contract from ProxyFactory in `distributeByOwner`

klaus

Summary

It does not verify that the proxy parameter is actually a proxy deployed at the protocol. So arbitrary contract call is possible.

Vulnerability Details

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

It do not check whether the contract received as the proxy parameter is actually a proxy created using salt. So the proxy can be any contract.

function _distribute(address proxy, bytes calldata data) internal {
(bool success,) = proxy.call(data);
if (!success) revert ProxyFactory__DelegateCallFailed();
emit Distributed(proxy, data);
}

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

_distribute makes low-level calls to the proxy. Therefore, arbitrary functions can be called by passing arbitrary parameters to arbitrary contracts.

Impact

Can do arbitrary contract call from ProxyFactory

Tools Used

vscode

Recommendations

Do not use the proxy parameter, but use the address obtained by calling getProxyAddress.

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

Support

FAQs

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