hats-finance / Tapioca--Lending-Engine--0x5bee198f5b060eecd86b299fdbea6b0c07c728dd

Other
0 stars 0 forks source link

Incorrect Transfer Condition in transfer Function #5

Open hats-bug-reporter[bot] opened 1 month ago

hats-bug-reporter[bot] commented 1 month ago

Github username: @Jelev123 Twitter username: zhulien_zhelev Submission hash (on-chain): 0x69d11623cf0dd5ca8c72d944b503f95b0fca35850eaaa3f3ab8f19de80e93184 Severity: medium

Description: Description\ The transfer function is designed to handle token transfers between addresses in a Solidity smart contract. It should ensure that transfers only occur when the amount to be transferred is non-zero and the sender is not transferring tokens to themselves.

The current implementation of the condition in the transfer function is incorrect. The condition if (amount != 0 || msg.sender == to) is intended to prevent transfers when the amount is zero and when the sender is transferring tokens to themselves. However, this logic is flawed. Specifically, the condition allows the function to proceed even if the amount is zero, provided that msg.sender is equal to to, which is not the intended behavior.

Attack Scenario\

An attacker or user can exploit this vulnerability by calling the transfer function with an amount of zero while setting to to be the same as msg.sender. This would allow the function to proceed, potentially executing unintended transfer logic even though no actual transfer should occur.

  1. Proof of Concept (PoC) File

transfer

Recommendation to fix

 function transfer(address to, uint256 amount) external virtual returns (bool) {
        // If `amount` is 0, or `msg.sender` is `to` nothing happens
        if (amount != 0 || msg.sender != to) {
            uint256 srcBalance = balanceOf[msg.sender];
            require(srcBalance >= amount, "ERC20: balance too low");
            if (msg.sender != to) {
                require(to != address(0), "ERC20: no zero address"); // Moved down so low balance calls safe some gas

                balanceOf[msg.sender] = srcBalance - amount; // Underflow is checked
                balanceOf[to] += amount;
            }
        }
        emit Transfer(msg.sender, to, amount);
        return true;
    }
cryptotechmaker commented 1 month ago

Have you seen this check ? if (msg.sender != to)

Jelev123 commented 1 month ago

// Ifamountis 0, ormsg.senderistonothing happens what does it means?

Jelev123 commented 4 weeks ago

??