Sparkn

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

Distributor.sol#_distribute() - If an address accidently sends tokens to the contract, the owner won't be able to retrieve and send back all of his tokens

0xdeth

Summary

If an address accidently sends tokens to the contract, the owner won't be able to retrieve and send back all of his tokens.

Vulnerability Details

If an address sends tokens to the Proxy by accident or if the sponsor accidently sends more tokens to the Proxy after the rewards have been distributed, the owner won't be able to send all of their tokens back. This occurs because of this if statement. The if check states that the totalPercentage MUST equal 10000 - COMMISSION_FEE. Because of this it's impossible to send the full 100% of tokens that are in the Proxy to a certain address as some tokens ALWAYS go to the STADIUM_ADDRESS.

In addition the STADIUM_ADDRESS can be a contract and if the contract only sends tokens to a specific address (the owner for example), the owner will then have to manually calculate how much tokens he has to send to the address that sent the original tokens by accident. Thus he will have to pay more gas, because he has to do more transactions, which is a small hit to his funds, but a hit nonetheless.

The exact same issue will occur if the sponsor and/or organizer send funds to the Proxy, but no suporter shows up to solve their problem. In this scenario the sponsor and/or organizer will suffer the 5% loss.

POC

The POC just shows the first scenario as both are basically the same.
Slightly modified version of setUp().

// A slightly modified version of setUp()
// We just mint some tokens for user2 here.
function setUp() public {
// set up balances of each token belongs to each user
if (block.chainid == 31337) {
// deal ether
vm.deal(factoryAdmin, STARTING_USER_BALANCE);
vm.deal(sponsor, SMALL_STARTING_USER_BALANCE);
vm.deal(organizer, SMALL_STARTING_USER_BALANCE);
vm.deal(user1, SMALL_STARTING_USER_BALANCE);
vm.deal(user2, SMALL_STARTING_USER_BALANCE);
vm.deal(user3, SMALL_STARTING_USER_BALANCE);
vm.deal(attacker, SMALL_STARTING_USER_BALANCE);
vm.deal(TEST_SIGNER, SMALL_STARTING_USER_BALANCE);
// mint erc20 token
vm.startPrank(tokenMinter);
MockERC20(jpycv1Address).mint(sponsor, 100_000 ether); // 100k JPYCv1
MockERC20(jpycv2Address).mint(sponsor, 300_000 ether); // 300k JPYCv2
MockERC20(usdcAddress).mint(sponsor, 10_000 ether); // 10k USDC
MockERC20(jpycv1Address).mint(organizer, 100_000 ether); // 100k JPYCv1
// Add this to the setUp function
MockERC20(jpycv2Address).mint(user2, 10 ether); // 10 JPYCv1
MockERC20(jpycv2Address).mint(organizer, 300_000 ether); // 300k JPYCv2
MockERC20(usdcAddress).mint(organizer, 10_000 ether); // 10k USDC
MockERC20(jpycv1Address).mint(TEST_SIGNER, 100_000 ether); // 100k JPYCv1
MockERC20(jpycv2Address).mint(TEST_SIGNER, 300_000 ether); // 300k JPYCv2
MockERC20(usdcAddress).mint(TEST_SIGNER, 10_000 ether); // 10k USDC
vm.stopPrank();
}
​
// labels
vm.label(organizer, "organizer");
vm.label(sponsor, "sponsor");
vm.label(supporter, "supporter");
vm.label(user1, "user1");
vm.label(user2, "user2");
vm.label(user3, "user3");
}

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

// Function we use to create the retrieval data for user2
function createRetrieveData() public returns(bytes memory data) {
address[] memory tokens_ = new address[](1);
tokens_[0] = jpycv2Address;
address[] memory winners = new address[](1);
// We set the winner, to user2, who sent tokens to the proxy by mistake
winners[0] = user2;
uint256[] memory percentages_ = new uint256[](1);
// We can't set the percentages to 10000,
// because of the COMMISION_FEE check inside the _distribute function
percentages_[0] = 9500;
data = abi.encodeWithSelector(Distributor.distribute.selector, jpycv2Address, winners, percentages_, "");
}
​
// The actual test
function testOwnerWontBeAbleToRecoverAllDust() public setUpContestForJasonAndSentJpycv2Token(organizer) {
// before
assertEq(MockERC20(jpycv2Address).balanceOf(user1), 0 ether);
assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 0 ether);
​
bytes32 randomId_ = keccak256(abi.encode("Jason", "001"));
bytes memory data = createData();
​
vm.warp(9 days); // 9 days later
vm.startPrank(organizer);
proxyFactory.deployProxyAndDistribute(randomId_, address(distributor), data);
vm.stopPrank();
​
// after
assertEq(MockERC20(jpycv2Address).balanceOf(user1), 9500 ether);
assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 500 ether);
​
// Calculate the proxy address
bytes32 salt = keccak256(abi.encode(organizer, randomId_, address(distributor)));
address proxyAddress = proxyFactory.getProxyAddress(salt, address(distributor));
// user2 accidently sends tokens to the proxy
vm.prank(user2);
MockERC20(jpycv2Address).transfer(proxyAddress, 10 ether);
​
assertEq(MockERC20(jpycv2Address).balanceOf(user2), 0 ether);
​
uint256 stadiumBalanceBefore = MockERC20(jpycv2Address).balanceOf(stadiumAddress);
​
// Expiration time passes
vm.warp(block.timestamp + 7 days);
// The factoryAdmin (owner) sees that there is dust in the Proxy contract
// and attempts to send it back, but he won't send all the tokens back to user2
// because of the hardcoded COMMISION_FEE, which always goes to the STATIUM_ADDRESS
// If the STADIUM_ADDRESS is a contract and it doesn't have a 'transfer' function which can be called
// TO a specific address, whatever address received the tokens from STADIUM_ADDRESS must calculate and send them
// back manually to user2.
bytes memory retrievalData = createRetrieveData();
vm.prank(factoryAdmin);
proxyFactory.distributeByOwner(proxyAddress, organizer, randomId_, address(distributor), retrievalData);
​
// The user doesn't receive all of his tokens back
assertFalse(MockERC20(jpycv2Address).balanceOf(user2) == 10 ether);
assertEq(MockERC20(jpycv2Address).balanceOf(user2), 95 * 1e17);
uint256 stadiumBalanceAfter = MockERC20(jpycv2Address).balanceOf(stadiumAddress);
​
assertGt(stadiumBalanceAfter, stadiumBalanceBefore);
}

Impact

Inconvenience for the user that receives the fee's and a small loss of funds, as he has to pay for the gas fees for the extra transaction that he has to do. This is assuming that the STADIUM_ADDRESS is a contract and that it has a type of claim/redeem function. If the STADIUM_ADDRESS has does something else with the tokens and they aren't directly sent to an EOA or another contract that can handle tokens, this issue can lead to a loss of funds for the user that accidently sent the tokens.

Because of this, I leave the severity at Medium.

Tools Used

Manual review
Foundry

Recommendations

Add a new function in Distributor.sol, we can call it distributeNoFees. It's going to basically be the exact same function, but it will only be called by the owner of the ProxyFactory. We can achieve, this using tx.origin and comparing him with the owner of the ProxyFactory.

Or alternatively make COMISSION_FEE not constant and add a setter in Distributor which can be called by the owner of the ProxyFactory or add Ownable to Distributor and add the setter there.

The modified function distribute and the new function to add in Distributor.sol.

function distribute(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
external
{
if (msg.sender != FACTORY_ADDRESS) {
revert Distributor__OnlyFactoryAddressIsAllowed();
}
// We always include fees if anyone else calls this function.
_distribute(token, winners, percentages, true, data);
}
​
// The new function to add.
function distributeNoFees(address token, address[] memory winners, uint256[] memory percentages, bytes memory data)
external
{
if (msg.sender != FACTORY_ADDRESS || tx.origin != ProxyFactory(FACTORY_ADDRESS).owner()) {
revert Distributor__OnlyFactoryAddressIsAllowed();
}
// Since only the `ProxyFactory` owner can call this function, we set `includeFees` to `false`.
_distribute(token, winners, percentages, false, data);
}

The modified version of _distribute.

function _distribute(address token, address[] memory winners, uint256[] memory percentages, bool includeFees, bytes memory data)
internal
{
// token address input check
if (token == address(0)) revert Distributor__NoZeroAddress();
if (!_isWhiteListed(token)) {
revert Distributor__InvalidTokenAddress();
}
// winners and percentages input check
if (winners.length == 0 || winners.length != percentages.length) revert Distributor__MismatchedArrays();
uint256 percentagesLength = percentages.length;
uint256 totalPercentage;
for (uint256 i; i < percentagesLength;) {
totalPercentage += percentages[i];
unchecked {
++i;
}
}
// check if totalPercentage is correct and we include fees
if (includeFees) {
if (totalPercentage != (10000 - COMMISSION_FEE)) {
revert Distributor__MismatchedPercentages();
}
}
....

The following are tests to demonstrate the new functionality.
Paste the following inside test/ProxyFactoryTest.t.sol and run forge test --mt testOwnerIsAbleToRecoverAllDust -vvvv.

// Function we use to create the retrieval data for user2 with NO FEES
function createRetrieveDataWithNoFeesSelector() public returns(bytes memory data) {
address[] memory tokens_ = new address[](1);
tokens_[0] = jpycv2Address;
address[] memory winners = new address[](1);
// We set the winner, to user2, who sent tokens to the proxy by mistake
winners[0] = user2;
uint256[] memory percentages_ = new uint256[](1);
// We can now set the percentages to the maximum.
percentages_[0] = 10000;
// Here we use the Distributor.distributeNoFees.selector.
data = abi.encodeWithSelector(Distributor.distributeNoFees.selector, jpycv2Address, winners, percentages_, "");
}
​
// The actual test
function testOwnerIsAbleToRecoverAllDust() public setUpContestForJasonAndSentJpycv2Token(organizer) {
// before
assertEq(MockERC20(jpycv2Address).balanceOf(user1), 0 ether);
assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 0 ether);
​
bytes32 randomId_ = keccak256(abi.encode("Jason", "001"));
bytes memory data = createData();
​
vm.warp(9 days); // 9 days later
vm.startPrank(organizer);
proxyFactory.deployProxyAndDistribute(randomId_, address(distributor), data);
vm.stopPrank();
​
// after
assertEq(MockERC20(jpycv2Address).balanceOf(user1), 9500 ether);
assertEq(MockERC20(jpycv2Address).balanceOf(stadiumAddress), 500 ether);
​
// Calculate the proxy address
bytes32 salt = keccak256(abi.encode(organizer, randomId_, address(distributor)));
address proxyAddress = proxyFactory.getProxyAddress(salt, address(distributor));
// user2 accidently sends tokens to the proxy
vm.prank(user2);
MockERC20(jpycv2Address).transfer(proxyAddress, 10 ether);
​
assertEq(MockERC20(jpycv2Address).balanceOf(user2), 0 ether);
​
uint256 stadiumBalanceBefore = MockERC20(jpycv2Address).balanceOf(stadiumAddress);
​
// Expiration time passes
vm.warp(block.timestamp + 7 days);
// The factoryAdmin (owner) can now call distributeNoFees and ONLY he can call it
bytes memory retrievalData = createRetrieveDataWithNoFeesSelector();
vm.prank(factoryAdmin, factoryAdmin);
proxyFactory.distributeByOwner(proxyAddress, organizer, randomId_, address(distributor), retrievalData);
​
// The user receives all of his tokens back
assertEq(MockERC20(jpycv2Address).balanceOf(user2), 10 ether);
uint256 stadiumBalanceAfter = MockERC20(jpycv2Address).balanceOf(stadiumAddress);
​
assertEq(stadiumBalanceAfter, stadiumBalanceBefore);
}

Support

FAQs

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