Lucene search

K
code423n4Code4renaCODE423N4:2022-08-MIMO-FINDINGS-ISSUES-154
HistoryAug 07, 2022 - 12:00 a.m.

Inadequate transferOwnership function prevents new owner from accessing funds

2022-08-0700:00:00
Code4rena
github.com
2
mimoproxy
ownership transfer
access control
action contracts
vault funds
vulnerability

Lines of code
<https://github.com/code-423n4/2022-08-mimo/blob/main/contracts/proxy/MIMOProxyRegistry.sol#L33&gt;
<https://github.com/code-423n4/2022-08-mimo/blob/main/contracts/proxy/MIMOProxy.sol#L116-L124&gt;

Vulnerability details

Description

There are two sources of truth for identifying the account that owns a given MIMOProxy:

  1. MIMOProxy.owner()
  2. MIMOProxyRegistry.getCurrentProxy(address)

The first source of truth is only used within the proxy contract itself, and the second source of truth is used to identify & authenticate the MIMOProxy associated with a given owner address.

MIMOProxy contains a function transferOwnership, which is intended to transfer control of the proxy to a new account. The transferOwnership function updates the MIMOProxy.owner() source of truth, but fails to update the MIMOProxyRegistry.getCurrentProxy source of truth.

Impact

The new owner of the proxy is unable to use the various “action” contract implementations to manage the proxy’s vaults.

This means the vault’s funds will be effectively locked. To recover the funds, the new owner will have to either transfer ownership back to the old address, or deploy a specially crafted action contract to recover the funds, then set up the contract using setPermission.

Fortunately, the old owner cannot access funds due to checks against the first source of truth in MIMOProxy.execute()

However, it should be noted that the old owner can set up a MIMOAutoAction after the proxy has been transferred to the new owner. MIMOAutoAction’s setAutomation function can be called from a non-proxy msg.sender, bypassing the first source of truth’s authorization check.

At this time, it does not appear that setting up MIMOAutoAction can cause a loss of funds for the vault.

Proof of Concept

High level PoC:

  1. User A creates a proxy via MIMOProxyRegistry.deploy()
  2. User A funds vault
  3. User A transfers ownership of the proxy to User B using MIMOProxy.transferOwnership

Result:

  • User B can no longer use any of the deployed action contracts to manage vault funds
  • User A can still manage the vault’s automation status using MIMOAutoAction.setAutomation

A coded PoC based on MIMOEmptyVault.test.ts can be found here: <https://gist.github.com/bsamuels453/8deeb68eb45d5a6d1b049c94082b0a4b&gt;

Tools used

  • Hardhat

Recommended Mitigation

Changes to the proxy contract’s owner must be propagated to the proxy registry.

Since a proxy contract’s new owner may already have a proxy associated with their address, ownership transfers should revert if the new owner address is already associated with a different proxy.


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

👍 1 horsefacts reacted with thumbs up emoji

All reactions

  • 👍 1 reaction