Christmas Dinner
First Flight #31
Beginner FriendlyFoundrySolidity
100 EXP
View results
Previous Next
Submission Details
Severity: high
Valid

User can't refund if he/she funds with a previously deposited token by him/her after a host withdraw

jimpixjed

Summary

If the user has deposited tokens, then the host has withdrawn them and then the user deposits more tokens and tries to withdraw we would get a ERC20InsufficientBalance error.

Vulnerability Details

When the host calls the withdrawfunction we do not reset the deposit amount for any of the users in the contract, but we send all of the balance for each token in the contract to the host. This would mean that if an user deposits more tokens in the future and tries to refund he/she wouldn't be able to do so, as the refund would try the transfer oldAmountOfTokens + newAmountDeposited back to the user, but the oldAmountOfTokens is already transfered to the host.

function withdraw() external onlyHost {
address _host = getHost();
i_WETH.safeTransfer(_host, i_WETH.balanceOf(address(this)));
i_WBTC.safeTransfer(_host, i_WBTC.balanceOf(address(this)));
i_USDC.safeTransfer(_host, i_USDC.balanceOf(address(this)));
}
function _refundERC20(address _to) internal {
i_WETH.safeTransfer(_to, balances[_to][address(i_WETH)]);
i_WBTC.safeTransfer(_to, balances[_to][address(i_WBTC)]);
i_USDC.safeTransfer(_to, balances[_to][address(i_USDC)]);
balances[_to][address(i_USDC)] = 0;
balances[_to][address(i_WBTC)] = 0;
balances[_to][address(i_WETH)] = 0;
}

Impact

User is not able to get any of his tokens newly invested tokens out of the contract if in the past he/she has invested the same tokens in the contract and the host has executed a withdraw.

Tools Used

  • Manual Review

  • Foundry Tests

POC

function test_userCannotRefundIfHostHasWithdrawnFunds() public {
uint256 wbtcAmount;
vm.startPrank(user1);
cd.deposit(address(wbtc), 0.5e18);
wbtcAmount += 0.5e18;
vm.startPrank(deployer);
cd.withdraw();
vm.stopPrank();
assertEq(wbtc.balanceOf(deployer), wbtcAmount);
​
vm.startPrank(user1);
//User deposits more WBTC
cd.deposit(address(wbtc), 0.5e18);
wbtcAmount += 0.5e18;
//User tries to refund his WBTC but he/she can't as the refund logic is trying to transfer more funds than the contract has as the host has called withdraw
cd.refund();
}

Recommendations

Either make the withdraw to be callable only after the deadline of the contract has passed, or reset funds for user whenever the host calls a withdraw(might waste more gas in this case as storage mappings need to be updated).

function withdraw() external onlyHost {
address _host = getHost();
​
// Sweep all contract balances to the host
i_WETH.safeTransfer(_host, i_WETH.balanceOf(address(this)));
i_WBTC.safeTransfer(_host, i_WBTC.balanceOf(address(this)));
i_USDC.safeTransfer(_host, i_USDC.balanceOf(address(this)));
​
// Clear all user balances
for (uint256 i = 0; i < balanceHolders.length; i++) {
address user = balanceHolders[i];
balances[user][address(i_WETH)] = 0;
balances[user][address(i_WBTC)] = 0;
balances[user][address(i_USDC)] = 0;
etherBalance[user] = 0;
}
​
}
Updates

Lead Judging Commences

0xtimefliez Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

withdraw is callable before deadline ends

Support

FAQs

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

Give us feedback!