sherlock-audit / 2023-02-surge-judging

4 stars 1 forks source link

gryphon - Risk of transfering tokens on behaf of somebody - risk of loss of tokens #246

Closed github-actions[bot] closed 1 year ago

github-actions[bot] commented 1 year ago

gryphon

high

Risk of transfering tokens on behaf of somebody - risk of loss of tokens

Summary

The pool has an external function transferFrom which allows any spender to transfer on behalf of anybody. It lacks checking for the allowance.

Vulnerability Detail

1 - The function transferFrom is external. 2 - There is an approve function on t he contract that stores the allowance for an spender to spend on behalf of someone. 3 - But on the function transferFrom the allowance is never checked before the transfer takes place. 4 - Anyone can call transferFrom and make a transfer from an address from to the address to for all the amount that from has.

Impact

Anyone can transfer Pool Tokens on behalf of anybody. It can cause huge loss of funds.

Code Snippet

Pool.sol#L284

    function transferFrom(address from, address to, uint amount) external returns (bool) { 
        require(to != address(0), "Pool: to cannot be address 0");
        allowance[from][msg.sender] -= amount;
        balanceOf[from] -= amount;
        unchecked {
            balanceOf[to] += amount;
        }
        emit Transfer(from, to, amount);
        return true;
    }

Tool used

Manual Review

Recommendation

Before making the transfer (i.e. updating the balances), check for the allowances amount first. That can be done by adding an require statement: require( allowance[from][msg.sender] > amount, "spender not allowed to transfer that amount"); like described below:

    function transferFrom(address from, address to, uint amount) external returns (bool) { 
       require(to != address(0), "Pool: to cannot be address 0");
+      require( allowance[from][msg.sender] > amount, "spender not allowed to transfer that amount");
       allowance[from][msg.sender] -= amount;
       balanceOf[from] -= amount;
       unchecked {
            balanceOf[to] += amount;
        }
        emit Transfer(from, to, amount);
        return true;
    }