sherlock-audit / 2024-02-tapioca-judging

3 stars 2 forks source link

duc - Unclaimed native tokens in TOFT contract can be stolen #107

Closed sherlock-admin2 closed 4 months ago

sherlock-admin2 commented 4 months ago

duc

medium

Unclaimed native tokens in TOFT contract can be stolen

Summary

See vulnerability detail

Vulnerability Detail

TOFT contract has a function to rescue the stuck native tokens in contract.

function rescueEth(uint256 amount, address to) external onlyOwner {
    (bool success,) = to.call{value: amount}("");
    if (!success) revert TOFT_Failed();
}

However, the wrap function doesn't check if the msg.value is equal to _amount. Therefore, an attacker can call wrap with msg.value as 0 and _amount as the ETH balance of the contract to steal the existing native tokens in this contract.

function wrap(address _fromAddress, address _toAddress, uint256 _amount)
        external
        payable
        whenNotPaused
        nonReentrant
        onlyHostChain
        returns (uint256 minted)
    {
        if (erc20 == address(0)) {
            _wrapNative(_toAddress, _amount, 0);
        } ...
    }
function _wrapNative(address _toAddress, uint256 _amount, uint256 _feeAmount) internal virtual {
    vault.depositNative{value: _amount}();
    _mint(_toAddress, _amount - _feeAmount);
}

Impact

Unclaimed native tokens in TOFT contract will be stolen

Code Snippet

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/Market.sol#L419-L422

Tool used

Manual Review

Recommendation

Should check if msg.value is equal to _amount in the case of wrapping native tokens:

if (erc20 == address(0)) {
    require(msg.value == _amount);
    _wrapNative(_toAddress, _amount, 0);
}
cryptotechmaker commented 4 months ago

Low/Invalid. The contract is not supposed to hold funds (TOFTVault is responsible for that). rescueEth is added just in case we need to save airdropped funds

sherlock-admin3 commented 4 months ago

1 comment(s) were left on this issue during the judging contest.

WangAudit commented:

as I understand tokens shouldn't be there (therefore there is a rescue function (not claim) in case anyone accidentally send tokens there; basically user mistake)

cryptotechmaker commented 3 months ago

Duplicate of https://github.com/sherlock-audit/2024-02-tapioca-judging/issues/22

nevillehuang commented 3 months ago

Invalid, token contracts are not supposed to hold funds, any accidental funds sent would be user mistake not accepted based on sherlock rules

  1. Users sending ETH/native tokens accidentally just because a contract allows is not a valid medium/high.