📄

Report #24107

Report Date
September 14, 2023
Status
Closed
Payout

Weak access control in function claimFertilized leads to theft of unclaimed yield.

‣
Report Info

Report ID

#24107

Report type

Smart Contract

Has PoC?

Yes

Target

Impacts

Theft of unclaimed yield

Bug Description

The function sends fertilized tokens without validating users. This makes it vulnerable to attacks. Attackers can exploit the function's weak access control to steal fertilized tokens because there are no access control checks in place.

function claimFertilized(uint256[] calldata ids, LibTransfer.To mode)
    external
    payable
{
    uint256 amount = C.fertilizer().beanstalkUpdate(msg.sender, ids, s.bpf);
    LibTransfer.sendToken(C.bean(), amount, msg.sender, mode);
}

Impact

The impact on the project is theft of unclaimed yield. By sending fertilized tokens without validating users, attackers can call the function to steal fertilized tokens.

Risk Breakdown

The lack of access control checks makes the function vulnerable to attacks. Attackers can call the function to steal fertilized tokens because there are no checks to validate users before transferring fertilized tokens.

Difficulty to Exploit: Easy Weakness: CVSS2 Score: 4

Recommendation

Use access control checks to validate users before transferring fertilized tokens.

function claimFertilized(uint256[] calldata ids, LibTransfer.To mode, address user) external payable {
    require(msg.sender == user, "Unauthorized");
    uint256 amount = C.fertilizer().beanstalkUpdate(user, ids, s.bpf);
    LibTransfer.sendToken(C.bean(), amount, user, mode);
}

References

Proof of concept

A proof of concept to demonstrate the vulnerability in Function claimFertilized.

Setup: Foundry, FertilizerFacet address, the Attacker's contract and test contract.

Run this in the console to get the necessary contracts needed to run the poc.

cast etherscan-source 0xFC7Ed192a24FaB3093c8747c3DDBe6Cacd335B6C -d src/external

Attacker's Contract

pragma solidity 0.7.6;

import "forge-std/console.sol";

import "../FertilizerFacet/contracts/libraries/Token/LibTransfer.sol";

contract Attack { address target; uint256[] public ids;

constructor(address fertilizedFaucet) public {
    target = fertilizedFaucet;
}

function attackCLaim() external {
    ids = new uint256[](2);
    ids[0] = 1;
    ids[1] = 2;

    // mode set to LibTransfer.To.INTERNAL
    LibTransfer.To mode = LibTransfer.To.INTERNAL;

    // Call function claimFertilized
    target.call(abi.encodeWithSelector(bytes4(keccak256(abi.encode("claimFertilized(uint256, enum)", ids, mode)))));

    // Get the balance of the attacker (msg.sender)
    uint256 attackerBalance = msg.sender.balance;
    console.log("Attacker's balance after attack: %s", attackerBalance);

}

}

FertilizerFacet Address

Test Contract

// SPDX-License-Identifier: MIT pragma solidity ^0.7.6;

import "forge-std/Test.sol";

import "../../src/reentrancy/examples/AT.sol";

contract AttackTest is Test {

Attack public attackContract;
address Fertilizer = address(0xFC7Ed192a24FaB3093c8747c3DDBe6Cacd335B6C);

function setUp() public {
    // Deploy the attack contract
    attackContract = new Attack(address(Fertilizer));
}

function testAttackClaim() public {

    // Call the attack function
    attackContract.attackCLaim();
}

}

Run this in the console to test the poc: forge test -vv --match-path test/A.t.sol

Replace A.t.sol with the name of your test contract

Result

Running 1 test for test/A.t.sol:AttackTest [PASS] testAttackClaim() (gas: 82223) Logs: Attacker's balance after attack: 79228162514264337593543950335

Test result: ok. 1 passed; 0 failed; 0 skipped; finished in 151.71ms Ran 1 test suites: 1 tests passed, 0 failed, 0 skipped (1 total tests)

BIC Response

This is not a valid but report as the POC does not showcase anything relevant. It is not proof of an issue that the attacker has a large Ether balance after calling claimFertilized. The claimFertilized function does not transfer any Ether (just Beans), so it's unclear why this evidence is used as a valid POC.

The recommended solution is simply to replace all instances of msg.sender with a new user argument that is validated to be the the same as msg.sender, so its unclear how this change would make a difference.

Due to these reasons, we are closing the submission and no reward will be issued.