DYAD - Angry_Mustache_Man's results

The first capital efficient overcollateralized stablecoin.

General Information

Platform: Code4rena

Start Date: 18/04/2024

Pot Size: $36,500 USDC

Total HM: 19

Participants: 183

Period: 7 days

Judge: Koolex

Id: 367

League: ETH

DYAD

Findings Distribution

Researcher Performance

Rank: 122/183

Findings: 2

Award: $7.32

🌟 Selected for report: 0

πŸš€ Solo Findings: 0

Lines of code

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

Vulnerability details

Impact

Denial of Service for the Victim User of withdraw & redeemDyad functions of VaultManagerV2.sol .

Proof of Concept

The current implementation of VaultManagerV2.sol#withdraw function has the following check before withdrawal from Vault where the state idToBlockOfLastDeposit is being checked as shown below :

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

  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 check is implemented to prevent flashloan attacks . But the problem arises when a Malicious user can simply deposit a small amount of token to the Vault to prevent the withdrawal process . This arises due to the fact that the VaultManagerV2.sol#deposit does not have a strict access modifier to prevent such attacks . Let's see how it can be executed . As shown below the VaultManagerV2.sol#deposit function has a lineant function access modifier which just checks whether the dnft used exists or not . And the function has this line of code where it changes the state idToBlockOfLastDeposit to current block.number as shown below.

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

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

The isValidDNft is shown below : https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L42C1-L44C4

  modifier isValidDNft(uint id) {
    if (dNft.ownerOf(id) == address(0))   revert InvalidDNft(); _;
  }

So whenever a Victim User tries to withdraw the asset's from the Vault , a Malicious User can frontrun the transaction and deposit 1 wei of token to the Vault to change the idToBlockOfLastDeposit to current block.number , thereby preventing the Victim User from withdrawing the funds .

Tools Used

Manual Review

To prevent this attack the access modifier for the deposit function should be made more strict . Either the access modifier should be changed from isValidDNft to isDNftOwner or the User should be allowed to create a allowlist of Trusted Users , who would be allowed to deposit on behalf of the DNFTOwner .

Assessed type

Access Control

#0 - c4-pre-sort

2024-04-27T11:54:07Z

JustDravee marked the issue as duplicate of #489

#1 - c4-pre-sort

2024-04-29T09:25:41Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-05T20:38:19Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#3 - c4-judge

2024-05-05T20:39:24Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#4 - c4-judge

2024-05-05T20:39:38Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#5 - c4-judge

2024-05-05T21:42:49Z

koolexcrypto marked the issue as nullified

#6 - c4-judge

2024-05-05T21:42:58Z

koolexcrypto marked the issue as not nullified

#7 - c4-judge

2024-05-08T15:27:55Z

koolexcrypto marked the issue as duplicate of #1001

#8 - c4-judge

2024-05-11T19:49:47Z

koolexcrypto marked the issue as satisfactory

#9 - c4-judge

2024-05-13T18:34:30Z

koolexcrypto changed the severity to 3 (High Risk)

Awards

7.3026 USDC - $7.30

Labels

bug
3 (High Risk)
satisfactory
sufficient quality report
:robot:_97_group
duplicate-128

External Links

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L205-L228 https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/VaultManagerV2.sol#L34-L35

Vulnerability details

Impact

Detailed description of the impact of this finding.

Proof of Concept

Let's see the implementation of the VaultManagerV2.sol#liquidate to see the calculation of the collateral earned by the Liquidator upon liquidation .

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

  function liquidate(
    uint id,
    uint to
  ) 
    external 
      isValidDNft(id)
      isValidDNft(to)
    {
      uint cr = collatRatio(id);
      if (cr >= MIN_COLLATERIZATION_RATIO) revert CrTooHigh();
      dyad.burn(id, msg.sender, dyad.mintedDyad(address(this), id));

      uint cappedCr               = cr < 1e18 ? 1e18 : cr;
      uint liquidationEquityShare = (cappedCr - 1e18).mulWadDown(LIQUIDATION_REWARD);
      uint liquidationAssetShare  = (liquidationEquityShare + 1e18).divWadDown(cappedCr);

      uint numberOfVaults = vaults[id].length();                                   <@audit
      for (uint i = 0; i < numberOfVaults; i++) {              
          Vault vault      = Vault(vaults[id].at(i));                              <@audit
          uint  collateral = vault.id2asset(id).mulWadUp(liquidationAssetShare);   <@audit
          vault.move(id, to, collateral);                                          <@audit
      }
      emit Liquidate(id, msg.sender, to);
  }

Here we can see that only the non-kerosene vaults of the DNFT's are considered . The kerosene vaults represented as vaultsKerosene is not considered . Check this code below where both kerosene and non-kerosene vaults are declared at the beginning of the code :

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

  mapping (uint => EnumerableSet.AddressSet) internal vaults; 
  mapping (uint => EnumerableSet.AddressSet) internal vaultsKerosene;

As per the official documentation given in the contest ReadMe , the liquidator must be given all the collateral including the collateral from kerosene vaults as shown below in the doc - Check the lines under the section - Liquidation and Redemption for the same : https://dyadstable.notion.site/DYAD-design-outline-v5-ed2d075eb691482d90cae262f822a2ff

The liquidating Note absorbs the liquidated Note’s DYAD balance and captures its collateral, including Kerosene.

Let's take an example to understand the situation . The MIN_COLLATERIZATION_RATIO = 1.5e18; // 150% in the protocol , let's say User has 160 USD worth of collateral in both Non-Kerosene & Kerosene Vaults and has minted 100 USD worth of dyad . Which implies the User to have collaterization ratio above 1.5e18 . But due to varying market conditions the value of collateral fell to 120 USD worth of collateral in Vaults where 80 USD worth of collateral was in Non-Kerosene Vault and 40 USD worth of collateral was in Kerosene Vaults . Now as the collaterization ratio fell below the minimum collaterization mark , the Liquidator comes to Liquidate the User and burns his 100 USD worth of DYAD for the same . The issue occurs as the Liquidator gets paid his reward only from Non-Kerosene vaults and not from Kerosene Vaults , as they were not included in the code implementation as shown above , causing heavy loses to the Liquidator .

Tools Used

Manual Review

Include Kerosene vaults in the calculation of collateral given to the Liquidator and move both Non-Kerosene & Kerosene Vaults colllateral to Liquidator's id to prevent losses for the Liquidator .

Assessed type

Math

#0 - c4-pre-sort

2024-04-28T10:23:53Z

JustDravee marked the issue as duplicate of #128

#1 - c4-pre-sort

2024-04-29T09:03:38Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-11T19:42:59Z

koolexcrypto marked the issue as satisfactory

AuditHub

A portfolio for auditors, a security profile for protocols, a hub for web3 security.

Built bymalatrax Β© 2024

Auditors

Browse

Contests

Browse

Get in touch

ContactTwitter