Report #13262

Unbounded gas consumption in `beanstalkMint()` leads to griefing

 function _delegate(address implementation) internal virtual {
        // solhint-disable-next-line no-inline-assembly
        assembly {
            // Copy msg.data. We take full control of memory in this inline assembly
            // block because it will not return to Solidity code. We overwrite the
            // Solidity scratch pad at memory position 0.
            calldatacopy(0, 0, calldatasize())

            // Call the implementation.
            // out and outsize are 0 because we don't know the size yet.
            let result := delegatecall(gas(), implementation, 0, calldatasize(), 0, 0)

            // Copy the returned data.
            returndatacopy(0, 0, returndatasize())

            switch result
            // delegatecall returns 0 on error.
            case 0 { revert(0, returndatasize()) }
            default { return(0, returndatasize()) }
        }
    }

BIC Response

Reporter Response

I think, Beanstalk team misunderstood the report a little, since the user is not required to mint any Fertilizer to contract, but instead the owner invokes beanstalkMint(address, uint, uint128, uint128) in order to mint ERC1155 tokens to arbitrary receiver.

Let's try to look again into the following code:

Copy

function __doSafeTransferAcceptanceCheck(
        address operator,
        address from,
        address to,
        uint256 id,
        uint256 amount,
        bytes memory data
    ) private {
        if (to.isContract()) {
            try IERC1155ReceiverUpgradeable(to).onERC1155Received(operator, from, id, amount, data) returns (bytes4 response) {
                if (response != IERC1155ReceiverUpgradeable.onERC1155Received.selector) {
                    revert("ERC1155: ERC1155Receiver rejected tokens");
                }
            } catch Error(string memory reason) {
                revert(reason);
            } catch {
                revert("ERC1155: transfer to non ERC1155Receiver implementer");
            }
        }
    }

Here we're invoking to.onERC1155Received(), where to - some user's SC. Basically, there are no any gas limits allocated to make this call happen and the user simply could drain the gas till the hard limit per block, therefore we can say for sure, that the impact listed on a Beanstalk main page (Unbounded gas consumption) makes sense in the context above.

Beanstalk team did a great job by simply providing the context where beanstalkMint() could be invoked. Unfortunately, FertilizerFacet.sol wasn't listed on Immunefi and I didn't see the clear context.

This report is now closed.

BIC Response

1. beanstalkMint() is only callable by the Fertilizer address, and is only used in line 58 of FertilizerFacet.sol: C.fertilizer().beanstalkMint(msg.sender, uint256(id), amount, s.bpf);. Therefore the receiver and the caller are the same person. Thus, while the receiver may very well do what is described in the bug, they would only be harming themselves.
2. This exploit could be harmful to the user if they increased the gas limit in their transaction, but this isn't something that Beanstalk can control.
3. While appreciate every bug report is appreciated, a POC is required, ideally one that forks mainnet and shows the economic damage. In the reported recommendation, it is explicitly stated that "users who are using smart contracts are fully aware of token-handling problems", meaning that in practice, the issue that was elevated is common practice in the space. In general, the use of mint() vs safeMint() is somewhat of a controversial argument (why do the additional computation to ensure the wallet is able to receive ERC1155's where 99% of the time they are able to), as Fertilizer is a temporary asset for Beanstalk, as well as it is a one-time transaction for many people, think that the reward outweighs the cost.