code-423n4 / 2023-10-opendollar-findings

10 stars 7 forks source link

initializeManager function can be frontrunned and eventually DOS SAFE minting ! #426

Open c4-submissions opened 10 months ago

c4-submissions commented 10 months ago

Lines of code

https://github.com/open-dollar/od-contracts/blob/v1.5.5-audit/src/contracts/proxies/Vault721.sol#L56

Vulnerability details

Impact

Detailed description of the impact of this finding.

Proof of Concept

Function initializeManager is for initializing the safemanager address of the vault contract . However lack of access control makes it vulnerable to fronrunning attacks . safemanager is the only authorized contract to mint safes in the vault contract .

 function initializeManager() external {
    if (address(safeManager) == address(0)) _setSafeManager(msg.sender);
  }

A malicious frontrunner can take advantages of this by below steps :

  1. At first he'll frontrun the initializeManager function to gain the authority to mint safes .
  2. call the build function to create a proxy for his account .
  3. Now , calling mint function and mint as many safes of any safeId as he want .

Although , Governance can regain control of safemanager by calling setSafeManager . But , This attack will DOS the minting of actual safemanager as some of the safes are previously minted by the attacker as SAFEs are minted sequentially .

Tools Used

Manual Review .

Recommended Mitigation Steps

Add a onlyGovernor modifier to the initializeManager funciton .

Assessed type

Access Control

c4-pre-sort commented 10 months ago

raymondfam marked the issue as low quality report

c4-pre-sort commented 10 months ago

raymondfam marked the issue as duplicate of #16

c4-judge commented 10 months ago

MiloTruck changed the severity to QA (Quality Assurance)

c4-judge commented 10 months ago

MiloTruck marked the issue as grade-b