code-423n4 / 2024-04-dyad-findings

8 stars 6 forks source link

Insecure Initialization Function Vulnerable to Frontrunning #113

Closed c4-bot-2 closed 4 months ago

c4-bot-2 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L59 https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/script/deploy/Deploy.V2.s.sol#L67

Vulnerability details

Impact

Gaining control of the KerosineManager contract through a vulnerability in VaultManagerV2 poses severe threats to the DeFi platform:

Proof of Concept

The Deploy.V2.s.sol is designed for setting up a complex smart contract ecosystem for the platform. It sequentially initializes and configures several contracts, including vaults for asset management and a KerosineManager for handling specific functionalities linked to these vaults.

Key Steps in the Script:

  1. Start Broadcasting: Begins transaction broadcast to deploy contracts and execute initial configuration functions.
  2. Deploy Licenser and VaultManagerV2: These contracts are foundational, with VaultManagerV2 managing the interactions and operational logistics of various vaults.
  3. Deploy Asset Vaults: It sets up specialized vaults (ethVault and wstEth) for managing different cryptocurrency assets.
  4. Initialize KerosineManager: A contract that presumably manages resources or permissions within the ecosystem.
  5. Associate KerosineManager with VaultManagerV2: Crucial step to link the KerosineManager with VaultManagerV2 for operational purposes.
  6. Deploy and Configure Additional Components: This includes deploying more vaults (UnboundedKerosineVault, BoundedKerosineVault) and setting their configurations, all the while linking back to the VaultManagerV2 and KerosineManager.
  7. Add Licenses and Transfer Ownerships: Finalizes the setup by adding licenses to the vaults and transferring ownership of certain contracts to a predefined owner.

Vulnerability Occurrence:

The potential vulnerability arises during the vaultManager.setKeroseneManager(kerosineManager); execution:

KerosineManager kerosineManager = new KerosineManager();
kerosineManager.add(address(ethVault));
kerosineManager.add(address(wstEth));

vaultManager.setKeroseneManager(kerosineManager);

At this juncture, the VaultManagerV2 contract has been deployed, and the script is in the process of configuring it with a KerosineManager. This step is critical because it establishes the operational relationship between the VaultManagerV2 and the KerosineManager, impacting how assets are managed within the ecosystem.

Analysis:

Given the sequential nature of the deployment script and the observable transactions on the blockchain, an attacker, monitoring the deployment, identifies the VaultManagerV2 deployment transaction. They can anticipate that the setKeroseneManager call will soon follow based on standard deployment patterns. The attacker can then preemptively make a call to vaultManager.setKeroseneManager with a malicious KerosineManager contract address, especially if there's no access control implemented in the setKeroseneManager function of VaultManagerV2.

This can be achieved by preparing a transaction targeting the VaultManagerV2.setKeroseneManager with the attacker's controlled KerosineManager, and broadcasting this transaction with a high gas fee to ensure it gets processed before the legitimate deployment script call:

// Assumed attacker's transaction crafted to run immediately after VaultManagerV2 is deployed
VaultManagerV2(vaultManagerAddress).setKeroseneManager(attackerControlledKerosineManager);

Since the legitimate script continues its deployment and configuration process, which includes deploying additional contracts and likely does not immediately verify the successful linkage of the KerosineManager, there's a substantial window of opportunity for the attacker's transaction to be mined first. Consequently, this compromises the integrity of the VaultManagerV2's setup by incorrectly associating it with a malicious KerosineManager, potentially leading to undesired operational control or other security vulnerabilities.

Tools Used

Manual Analysis

Recommended Mitigation Steps

To address the vulnerability allowing unauthorized control of the KerosineManager through the VaultManagerV2 setup process, the following mitigation steps are recommended:

Implementing Strict Access Control:

  1. Initializer Access Control: Enhance the access control for initialization functions such as setKeroseneManager in VaultManagerV2. Ensure that only the deployer or a specific authorized entity can call these functions. This can be achieved by implementing a modifier that checks the caller's address against a list of authorized addresses.

    • Example:
      modifier onlyDeployer() {
       require(msg.sender == deployerAddress, "Unauthorized");
       _;
      }

Securing Contract Initialization:

  1. Use Constructor for Critical Setup: Whenever possible, move critical setup configurations to the contract's constructor. This ensures that configurations like setting the KerosineManager are done atomically upon deployment, leaving no window for malicious interference.

    • Example:
      constructor(KerosineManager _kerosineManager) {
       kerosineManager = _kerosineManager;
      }
  2. One-time Initialization Pattern: For contracts that cannot have their setup moved entirely to the constructor, ensure the initialization function can only be successfully called once. This can be done by setting a state variable upon the first call and checking this variable at the beginning of the initialization function.

    • Example:

      bool private initialized = false;
      
      function initialize(KerosineManager _kerosineManager) external {
       require(!initialized, "Already initialized");
       kerosineManager = _kerosineManager;
       initialized = true;
      }

Assessed type

Access Control

c4-pre-sort commented 4 months ago

JustDravee marked the issue as insufficient quality report

c4-pre-sort commented 4 months ago

JustDravee marked the issue as duplicate of #1056

c4-pre-sort commented 4 months ago

JustDravee marked the issue as remove high or low quality report

c4-pre-sort commented 4 months ago

JustDravee marked the issue as sufficient quality report

c4-judge commented 4 months ago

koolexcrypto marked the issue as unsatisfactory: Out of scope