Sparkn

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

Organizer can cheat Winners with Wrong Distribution data

shanb1605

Summary

Once Distributor Proxy is deployed the Distributor::distribute() is called through Factory::_distribute(proxy,data).The issue relies on the data parameter as the organizer can give any data which included prize distribution to the wrong Winners or to himself steal all the contest funds.

Vulnerability Details

The Vulnerability that took place in the data parameter is Organizer can call the Distributor contract with any malformed data that may intentionally or unintentionally distribute the prize to the wrong person.

function deployProxyAndDistribute(bytes32 contestId, address implementation, bytes calldata data)
public
returns (address)
{
bytes32 salt = _calculateSalt(msg.sender, contestId, implementation);
if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
// can set close time to current time and end it immediately if organizer wish
if (saltToCloseTime[salt] > block.timestamp) revert ProxyFactory__ContestIsNotClosed();
address proxy = _deployProxy(msg.sender, contestId, implementation);
_distribute(proxy, data);
return proxy;
}
​
function _distribute(address proxy, bytes calldata data) internal {
(bool success,) = proxy.call(data);
if (!success) revert ProxyFactory__DelegateCallFailed();
emit Distributed(proxy, data);
}

Even KYC can done for Organizers, but there is a good solution to prevent this bug which is Reffered in #Recommendation

Impact

Organizers can get Benefited from participants and cheat them with no distribution of prizes or wrongly distribute them.

Tools Used

Manual Review

Recommendations

After setting up the contest and before the deployment, the owner should add the Winner's Data mapping to the respective salt.
The code can be refactored by adding a new function to add Winner Data and used in the deployProxyAndDistribute() and other related functions. Also internal _distribure() is changed from bytes calldata to bytes memory data.

Here is the Fix:

mapping(bytes32 => bytes) public WinnerOf;
​
function addWinnerData(bytes32 salt,bytes memory data) public onlyOwner {
WinnerOf[salt] = data;
}
​
//Removed existing data parameter
function deployProxyAndDistribute(bytes32 contestId, address implementation)
public
returns (address)
{
bytes32 salt = _calculateSalt(msg.sender, contestId, implementation);
//Data is copied here
bytes memory data = WinnerOf[salt];
if (saltToCloseTime[salt] == 0) revert ProxyFactory__ContestIsNotRegistered();
// can set close time to current time and end it immediately if organizer wish
if (saltToCloseTime[salt] > block.timestamp) revert ProxyFactory__ContestIsNotClosed();
address proxy = _deployProxy(msg.sender, contestId, implementation);
//data is used here
_distribute(proxy, data);
return proxy;
}
​
//bytes memory is used instead of calldata
function _distribute(address proxy, bytes memory data) internal {
(bool success,) = proxy.call(data);
if (!success) revert ProxyFactory__DelegateCallFailed();
emit Distributed(proxy, data);
}

But When Setting up contests and Adding Winner's data there is the responsibility of the Owner to check the Winner's Data properly.

Support

FAQs

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