Christmas Dinner

First Flight #31
Beginner FriendlyFoundrySolidity
100 EXP
View results
Submission Details
Severity: medium
Invalid

If participant or host got blacklisted by asset contract, their funds will be permanently frozen in the contract

Summary

It's impossible for participant or host to transfer their otherwise withdrawable funds to another address. If for some reason these actors got blacklisted by one of the token contracts, these funds will be permanently frozen as now there is no mechanics to move them to another address or specify the recipient for the transfer.

Vulnerability Details

As we know, the USDC token is blacklistable which means that any address could be blacklisted by the governance body. Although the probability is low, the lack of a mechanism which allows an actor to specify a recipient address, which the tokens should be transfered to, is a serious vulnerability.

For example, assume that the deadline has passed and the signing up process for the event has finished successfully. Now, the host is able to withdrawal the deposited funds to prepare the Christmas event. However, if the host's address got blacklisted during the event, there would be no way to retrieve the collected funds.

The same goes for the participants. If one decides to refund their deposit and got blacklisted before that they will not be able to retrieve their funds.

Even though from the whitelisted tokens only USDC has blacklist the whole withdraw or refund transaction will revert locking the WETH and WBTC tokens as well.

Impact

The contract's funds can be permanently frozen.

Tools Used

Manual review

Recommendations

One solution could be to just add a recipient argument to the withdraw and refund functions, so the msg.sender can specify what address should receive the funds. Of course, then a check for valid participant and host must be implemented, so that not anyone can steal the funds.

Example:

  • for the withdraw() there already is an onlyHost() modifier that checks for the validity of the msg.sender. Adding an argument for recipient address is sufficient here.

- function withdraw() external onlyHost {
+ function withdraw(address _recipient) external onlyHost {
address _host = getHost();
- i_WETH.safeTransfer(_host, i_WETH.balanceOf(address(this)));
+ i_WETH.safeTransfer(_recipient, i_WETH.balanceOf(address(this)));
- i_WBTC.safeTransfer(_host, i_WBTC.balanceOf(address(this)));
+ i_WBTC.safeTransfer(_recipient, i_WBTC.balanceOf(address(this)));
- i_USDC.safeTransfer(_host, i_USDC.balanceOf(address(this)));
+ i_USDC.safeTransfer(_recipient, i_USDC.balanceOf(address(this)));
}

  • for the refund() adding an argument for recipient will not be enough. We have to change the implementation of the _refundERC20() as well.

    - function refund() external nonReentrant beforeDeadline {
    + function refund(address _recipient) external nonReentrant beforeDeadline {
    - address payable _to = payable(msg.sender);
    + address payable _from = payable(msg.sender);
    - _refundERC20(_to);
    + _refundERC20(_from, _recipient);
    - _refundETH(_to);
    + _refundETH(_from);
    emit Refunded(msg.sender);
    }

    Replace the _refundERC20() with the following:

    function _refundERC20(address _from, address _recipient) internal {
    i_WETH.safeTransfer(_recipient, balances[_from][address(i_WETH)]);
    i_WBTC.safeTransfer(_recipient, balances[_from][address(i_WBTC)]);
    i_USDC.safeTransfer(_recipient, balances[_from][address(i_USDC)]);
    balances[_from][address(i_USDC)] = 0;
    balances[_from][address(i_WBTC)] = 0;
    balances[_from][address(i_WETH)] = 0;
    }

Now, even if one of the actors got blacklisted their tokens will not be permanently frozen because they have the opportunity to choose a recipient address that is not blacklisted. It is essential to still use the msg.sender as a source address, so that there isn't a possibility for a malicious actor to just steal funds from legitimate users.

Updates

Lead Judging Commences

0xtimefliez Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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