Lines of code
<https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L245-L251&gt;
<https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/Market.sol#L246-L248&gt;
<https://github.com/code-423n4/2022-10-inverse/blob/3e81f0f5908ea99b36e6ab72f13488bbfe622183/src/escrows/SimpleERC20Escrow.sol#L25-L29&gt;

Vulnerability details

Impact

The user’s collateral is not held in the market contract but is instead held in individual escrows. Every user has a unique escrow for every market. And the escrow contracts are created via the Market contract’s createEscrow() function. And it’s initialized with the collateral and user parameters. However, the initialize function of the Escrow is public and there are no modifiers. It can be frontrun by an actor who monitors the mempool for the createEscrow func-sig. Hence, the actor steals the collateral assets of the user.

Proof of Concept

1. A market wants to deposit its collateral and calls the Market contract’s deposit function;

   function deposit(address user, uint amount) public {
   IEscrow escrow = getEscrow(user);
   collateral.transferFrom(msg.sender, address(escrow), amount);
   if(callOnDepositCallback) {
   escrow.onDeposit();
   }
   emit Deposit(user, amount);
   }

Permalink
2. The function checks whether the market has an escrow contract via getEscrow internally;

    function getEscrow(address user) internal returns (IEscrow) {
        if(escrows[user] != IEscrow(address(0))) return escrows[user];
        IEscrow escrow = createEscrow(user);
        escrow.initialize(collateral, user);
        escrows[user] = escrow;
        return escrow;
    }

Permalink

3. It’s the first time for the market so it returns with creating escrow via createEscrow and initializes the escrow via;

      if(escrows[user] != IEscrow(address(0))) return escrows[user];
      IEscrow escrow = createEscrow(user);
      escrow.initialize(collateral, user);
   

Permalink

4. The actor sees the TX in mempool and frontruns the initialize function as it’s public and has no modifier.

   function initialize(IERC20 _token, address) public {
   require(market == address(0), “ALREADY INITIALIZED”);
   market = msg.sender;
   token = _token;
   }

Permalink

Tools Used

Manual Review

Recommended Mitigation Steps

It’s recommended to add an initializer modifier to the initialize function.

---

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

All reactions