Sparkn

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

Owner can incorrectly pull funds from contests not yet expired

t0x1c

Summary

Owner can incorrectly pull funds from a closed contest which has not yet expired using distributeByOwner().

Vulnerability Details

The distributeByOwner() function has 5 parameters: proxy, organizer, contestId, implementation, data. However, there is no 'linkage' between proxy and the remaining params.

In order to check if the contest has expired, it uses bytes32 salt = _calculateSalt(organizer, contestId, implementation);. There is no check if this is indeed the salt of the proxy address. Hence, the owner can by mistake call distributeByOwner() with an incorrect proxy address of a contest which is closed, but not yet expired and drain its funds incorrectly.

PoC: (run via forge test --mt test_OwnerCanIncorrectlyPullFundsFromContestsNotYetExpired -vv)

function test_OwnerCanIncorrectlyPullFundsFromContestsNotYetExpired() public {
// Imagine that 2 contests are started by the same organizer & sponsor. This is just for
// simplicity; the organizers/sponsors can be considered as different too for the contests in question.
​
vm.startPrank(factoryAdmin);
bytes32 randomId_1 = keccak256(abi.encode("Jason", "015")); // contest_1
bytes32 randomId_2 = keccak256(abi.encode("Watson", "016")); // contest_2
proxyFactory.setContest(organizer, randomId_1, block.timestamp + 8 days, address(distributor));
proxyFactory.setContest(organizer, randomId_2, block.timestamp + 10 days, address(distributor));
vm.stopPrank();
​
bytes32 salt_1 = keccak256(abi.encode(organizer, randomId_1, address(distributor)));
address proxyAddress_1 = proxyFactory.getProxyAddress(salt_1, address(distributor));
bytes32 salt_2 = keccak256(abi.encode(organizer, randomId_2, address(distributor)));
address proxyAddress_2 = proxyFactory.getProxyAddress(salt_2, address(distributor));
​
vm.startPrank(sponsor);
// sponsor funds both his contests
MockERC20(jpycv2Address).transfer(proxyAddress_1, 10000 ether);
MockERC20(jpycv2Address).transfer(proxyAddress_2, 500 ether);
vm.stopPrank();
​
// before
assertEq(MockERC20(jpycv2Address).balanceOf(user1), 0 ether, "user1 balance not zero");
assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 0 ether, "STADIUM balance not zero");
assertEq(MockERC20(jpycv2Address).balanceOf(proxyAddress_1), 10000 ether, "proxy1 balance not 10000e18");
assertEq(MockERC20(jpycv2Address).balanceOf(proxyAddress_2), 500 ether, "proxy2 balance not 500e18");
​
bytes memory data = createData();
​
// 9 days later, organizer deploy and distribute -- for contest_1
vm.warp(9 days);
vm.prank(organizer);
proxyFactory.deployProxyAndDistribute(randomId_1, address(distributor), data);
// sponsor send token to proxy by mistake
vm.prank(sponsor);
MockERC20(jpycv2Address).transfer(proxyAddress_1, 11000 ether);
​
// 11 days later, organizer deploy and distribute -- for contest_2
vm.warp(11 days);
vm.prank(organizer);
proxyFactory.deployProxyAndDistribute(randomId_2, address(distributor), data);
// sponsor send token to proxy by mistake
vm.prank(sponsor);
MockERC20(jpycv2Address).transfer(proxyAddress_2, 600 ether);
​
// create data to send the token to admin
bytes memory dataToSendToAdmin = createDataToSendToAdmin();
​
// 16 days later from the start date, contest_1 has EXPIRED,
// but contest_2 is only CLOSED, not "EXPIRED".
// Hence, Owner should NOT be able to distribute rewards from funds reserved for contest_2.
vm.warp(16 days);
vm.prank(factoryAdmin);
// Owner provides `proxyAddress_2` by mistake, but remaining params are for `contest_1`
proxyFactory.distributeByOwner(proxyAddress_2, organizer, randomId_1, address(distributor), dataToSendToAdmin);
// above call should have reverted with "ProxyFactory__ContestIsNotExpired()"
​
// after
// STADIUM balance has now become (5% of 10000) + (5% of 500) + 600
assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 1125 ether, "STADIUM balance not 1125e18");
assertEq(MockERC20(jpycv2Address).balanceOf(proxyAddress_1), 11000 ether, "proxy1 balance not 11000e18");
// contest_2 is fully drained
assertEq(MockERC20(jpycv2Address).balanceOf(proxyAddress_2), 0, "proxy2 balance not zero");
}

The above is even more serious if Owner is trying to return the funds to sponsor1 using distributeByOwner(). Sponsor1 will get Sponsor2's funds (95% of funds, at the most).

OR

If the owner, upon a request from the sponsor, is trying to distribute contest_1's extra funds deposited by the sponsor as rewards to its winners. These winners would be completely different from the winners of contest_2, but funds from contest_2 will be redirected to "winner_1s".



Noteworthy is the fact that once any sponsor deposits extra funds by mistake later on (after proxy has been deployed via deployProxyAndDistribute() or similar functions & the rewards have been distributed once) he can only take the help of the owner to send the funds to any specific address(es).

Impact

  • Loss of funds as it can be drained by the owner by mistake from a not-yet-expired contest.

  • Funds/Rewards could be sent to incorrect sponsor/winners

  • Bypasses intended functionality.

Tools Used

Manual review, forge.

Recommendations

Add the following line inside distributeByOwner():

require(getProxyAddress(salt, implementation) == proxy);

Support

FAQs

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