Summary
There is a missing check in the ProxyFactory.setContest() function to see if the contestId parameter has already been used. It cause one digest can be used in different saltToCloseTime[salt] by ProxyFactory.deployProxyAndDistributeBySignature() function.
Vulnerability Details
Owner can use the same contestId to set several contests.
function setContest(address organizer, bytes32 contestId, uint256 closeTime, address implementation)
public
onlyOwner
{
if (organizer == address(0) || implementation == address(0)) revert ProxyFactory__NoZeroAddress();
if (closeTime > block.timestamp + MAX_CONTEST_PERIOD || closeTime < block.timestamp) {
revert ProxyFactory__CloseTimeNotInRange();
}
bytes32 salt = _calculateSalt(organizer, contestId, implementation);
if (saltToCloseTime[salt] != 0) revert ProxyFactory__ContestIsAlreadyRegistered();
saltToCloseTime[salt] = closeTime;
emit SetContest(organizer, contestId, closeTime, implementation);
}
The digest is generated by abi.encode(contestId, data). It means that if there has multiple contests use tha same contestId with different salt, the digest can be used on all of them.
function deployProxyAndDistributeBySignature(
address organizer,
bytes32 contestId,
address implementation,
bytes calldata signature,
bytes calldata data
) public returns (address) {
bytes32 digest = _hashTypedDataV4(keccak256(abi.encode(contestId, data)));
if (ECDSA.recover(digest, signature) != organizer) revert ProxyFactory__InvalidSignature();
bytes32 salt = _calculateSalt(organizer, contestId, implementation);
if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
if (saltToCloseTime[salt] > block.timestamp) revert ProxyFactory__ContestIsNotClosed();
address proxy = _deployProxy(organizer, contestId, implementation);
_distribute(proxy, data);
return proxy;
}
Impact
It will cause a situation that one contestId corresponeds to multiple contests. In other words, one contestId corresponds to multiple salt and saltToCloseTime[salt]. In contestId, one parameter should correspond to one salt and one saltToCloseTime[salt]. However, if there has multiple contests use tha same contestId with different salt, the digest can be used on all of them.
Tools Used
Manual
Recommendations
Recommend to fix the ProxyFactory.setContest() function as follow.
mapping(bytes32 => bool) public contestIdIsUsed;
function setContest(address organizer, bytes32 contestId, uint256 closeTime, address implementation)
public
onlyOwner
{
if (organizer == address(0) || implementation == address(0)) revert ProxyFactory__NoZeroAddress();
if (closeTime > block.timestamp + MAX_CONTEST_PERIOD || closeTime < block.timestamp) {
revert ProxyFactory__CloseTimeNotInRange();
}
if(contestIdIsUsed[contestId] == true){
revert();
}else{
contestIdIsUsed[contestId] = true;
}
bytes32 salt = _calculateSalt(organizer, contestId, implementation);
if (saltToCloseTime[salt] != 0) revert ProxyFactory__ContestIsAlreadyRegistered();
saltToCloseTime[salt] = closeTime;
emit SetContest(organizer, contestId, closeTime, implementation);
}
