DYAD - T1MOH'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: 15/183

Findings: 10

Award: $585.68

🌟 Selected for report: 0

🚀 Solo Findings: 0

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/src/core/KerosineManager.sol#L16

Vulnerability details

Impact

Currently Deploy script adds newly deployed WETH,wstETH vaults to VaultLicenser and KerosineManager. User can add the same vault twice to usual vaults and to kerosineVaults.

It is very easy to exploit: User adds the same vault twice and his deposit is twice valuable than it is. It allows to mint free DYAD which breaks the assumption that 1 DYAD is backed by at least 1 USD. As a result, protocols integrating DYAD can be exploited.

For example:

  1. User adds WETH vault as usual vault and kerosine vault.
  2. Suppose 1 WETH = 3000 USD. User deposits 10 WETH (30,000 USD).
  3. His collateral is calculated twice as 60,000 USD. Now he can mint up to 60,000 / 150% = 40,000 DYAD

Proof of Concept

In deploy script you can see that newly deployed WETH,wstETH vaults are added to VaultLicenser and KerosineManager:

  function run() public returns (Contracts memory) {
    ...

@>  kerosineManager.add(address(ethVault));
@>  kerosineManager.add(address(wstEth));

    vaultManager.setKeroseneManager(kerosineManager);

    ...

@>  vaultLicenser.add(address(ethVault));
@>  vaultLicenser.add(address(wstEth));
    ...
  }

And here you can see that the same WETH,wstETH can be registered twice:

  function add(
      uint    id,
      address vault
  ) 
    external
      isDNftOwner(id)
  {
    if (vaults[id].length() >= MAX_VAULTS) revert TooManyVaults();
@>  if (!vaultLicenser.isLicensed(vault))  revert VaultNotLicensed();
    if (!vaults[id].add(vault))            revert VaultAlreadyAdded();
    emit Added(id, vault);
  }

  function addKerosene(
      uint    id,
      address vault
  ) 
    external
      isDNftOwner(id)
  {
    if (vaultsKerosene[id].length() >= MAX_VAULTS_KEROSENE) revert TooManyVaults();
@>  if (!keroseneManager.isLicensed(vault))                 revert VaultNotLicensed();
    if (!vaultsKerosene[id].add(vault))                     revert VaultAlreadyAdded();
    emit Added(id, vault);
  }

Tools Used

Manual Review

Make two sets in KerosineManager:

  1. First for licensed Kerosine vaults
  2. Second for managed vaults with exogenous TVL

Assessed type

Other

#0 - c4-pre-sort

2024-04-29T05:26:58Z

JustDravee marked the issue as duplicate of #966

#1 - c4-pre-sort

2024-04-29T08:38:08Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-04T09:46:30Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#3 - c4-judge

2024-05-28T15:28:25Z

koolexcrypto marked the issue as duplicate of #1133

#4 - c4-judge

2024-05-29T07:06:41Z

koolexcrypto marked the issue as satisfactory

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/script/deploy/Deploy.V2.s.sol#L95

Vulnerability details

Impact

User can add Kerosine vault as usual exogenous vault via function add() because in Deploy script it's mistakenly licensed. It breaks the core invariant that every DYAD is backed by at least 1 USD of exogenous collateral, because now Kerosine can be used as single collateral

    if (getNonKeroseneValue(id) - value < dyadMinted) revert NotEnoughExoCollat();

Another impact is that UnboundedKerosineVault cannot be added in addKerosene() because it's not added to KerosineManager in Deploy script.

Proof of Concept

In deploy script you can see that unboundedKerosineVault is registered in vaultLicenser:

  function run() public returns (Contracts memory) {
    vm.startBroadcast();  // ----------------------

    Licenser vaultLicenser = new Licenser();

    // Vault Manager needs to be licensed through the Vault Manager Licenser
    VaultManagerV2 vaultManager = new VaultManagerV2(
      DNft(MAINNET_DNFT),
      Dyad(MAINNET_DYAD),
      vaultLicenser
    );

    ...

    vaultLicenser.add(address(ethVault));
    vaultLicenser.add(address(wstEth));
@>  vaultLicenser.add(address(unboundedKerosineVault));
@>  // vaultLicenser.add(address(boundedKerosineVault));

    ...
  }

However vaultLicenser is meant to contain exogenous vaults, so unboundedKerosineVault can be used as exogenous vault:

  function add(
      uint    id,
      address vault
  ) 
    external
      isDNftOwner(id)
  {
    if (vaults[id].length() >= MAX_VAULTS) revert TooManyVaults();
    if (!vaultLicenser.isLicensed(vault))  revert VaultNotLicensed();
    if (!vaults[id].add(vault))            revert VaultAlreadyAdded();
    emit Added(id, vault);
  }

Tools Used

Manual Review

Do not license Kerosine vaults in VaultLicenser, instead license them in separate KerosineVaultLicenser.

Assessed type

Other

#0 - c4-pre-sort

2024-04-29T05:43:40Z

JustDravee marked the issue as duplicate of #966

#1 - c4-pre-sort

2024-04-29T08:38:08Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-04T09:46:29Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#3 - c4-judge

2024-05-28T15:28:27Z

koolexcrypto marked the issue as duplicate of #1133

#4 - c4-judge

2024-05-29T07:06:46Z

koolexcrypto marked the issue as satisfactory

Findings Information

Awards

200.8376 USDC - $200.84

Labels

bug
3 (High Risk)
satisfactory
sufficient quality report
:robot:_174_group
duplicate-1097

External Links

Lines of code

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

Vulnerability details

Impact

Protocol uses only full liquidation, it doesn't allow to liquidate partially in comparison to usual lending protocols.

For example whale's position is very big and now becomes liquidateable. On liquidation whole position's DYAD debt must be repayed. To perform such a big liquidation, liquidator must borrow the same amount of DYAD, i.e. deposit the same amount as whale to protocol and repay on behalf of whale.

Usually flashloan is used to perform such liquidation, however in this protocol liquidator is unable to deposit and withdraw in the same block, so the only case for liquidator is to have free funds as much as whale's position to perform liquidation which reduces liquidation efficiency.

And this is where game theory comes into play: if liquidator has more funds than whale to perform that liquidation, then he becomes that non-liquidateable whale.

As a result, protocol will accumulate bad debt because of huge non-liquidateable position.

Proof of Concept

Here you can see that liquidator must repay liquidatee's whole debt.

  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));

      ...
  }

Here you can see that user can't withdraw at the same block as deposited last time. It means flashloan can't be used for liquidation

  function withdraw(
    uint    id,
    address vault,
    uint    amount,
    address to
  ) 
    public
      isDNftOwner(id)
  {
@>  if (idToBlockOfLastDeposit[id] == block.number) revert DepositedInSameBlock();
    ...
  }

Tools Used

Manual Review

Allow partial liquidation.

Assessed type

Other

#0 - c4-pre-sort

2024-04-28T10:05:06Z

JustDravee marked the issue as duplicate of #1097

#1 - c4-pre-sort

2024-04-29T08:34:32Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-11T12:22:11Z

koolexcrypto marked the issue as satisfactory

Lines of code

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

Vulnerability details

Impact

There is variable idToBlockOfLastDeposit to prevent deposit and withdraw in the same block. But attacker can frontrun user's withdrawal by depositing 1 wei to his Dnft. As a result, user's withdrawal will revert.

There is no limitation in this attack, attacker can block withdrawal for any user for any amount of time.

Proof of Concept

In deposit it only checks that DNft is minted, not only user can deposit to it

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

  function withdraw(
    uint    id,
    address vault,
    uint    amount,
    address to
  ) 
    public
      isDNftOwner(id)
  {
    if (idToBlockOfLastDeposit[id] == block.number) revert DepositedInSameBlock();
    ...
  }

Tools Used

Manual Review

Allow only user to deposit to DNft:

  function deposit(
    uint    id,
    address vault,
    uint    amount
  ) 
    external 
-     isValidDNft(id)
+     isDNftOwner(id)
  {
    ...
  }

Assessed type

DoS

#0 - c4-pre-sort

2024-04-27T11:56:40Z

JustDravee marked the issue as duplicate of #489

#1 - c4-pre-sort

2024-04-29T09:28:52Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-05T20:39:59Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#3 - c4-judge

2024-05-05T20:45:35Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#4 - c4-judge

2024-05-05T21:49:37Z

koolexcrypto marked the issue as nullified

#5 - c4-judge

2024-05-05T21:49:40Z

koolexcrypto marked the issue as not nullified

#6 - c4-judge

2024-05-08T15:26:42Z

koolexcrypto marked the issue as duplicate of #1001

#7 - c4-judge

2024-05-11T19:49:11Z

koolexcrypto marked the issue as satisfactory

Awards

3.8221 USDC - $3.82

Labels

bug
3 (High Risk)
satisfactory
sufficient quality report
edited-by-warden
:robot:_52_group
duplicate-308

External Links

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/script/deploy/Deploy.V2.s.sol#L64-L65

Vulnerability details

Impact

Kerosine price is determined by formula price = (TVL - DyadSupply) / KerosineSupply. Where TVL according to docs:

C: the total USD value of all exogenous collateral in the protocol (TVL). Critically, this total does not include Kerosene itself

Old vaults contain in total 1.8M USD and currently DYAD supply is 600_000 tokens. Problem is that after deploying V2 it will account for DYAD's current supply, but not account TVL in V1. As a result it will heavily underestimate Kerosine price.

Also it introduces attack vector:

  1. Attacker flashloans big amount, deposits in V1 and mints DYAD against it.
  2. Kerosine price falls down to 0.
  3. Attacker liquidates users and makes profit.

Proof of Concept

TVL is calculated by looping through kerosineManager's vaults. Note that it uses current DYAD supply which can be manipulated via flashloan in V1. So during flashloan attack numerator can be equal to 0.

  function assetPrice() 
    public 
    view 
    override
    returns (uint) {
      uint tvl;
      address[] memory vaults = kerosineManager.getVaults();
      uint numberOfVaults = vaults.length;
      for (uint i = 0; i < numberOfVaults; i++) {
@>      Vault vault = Vault(vaults[i]);
        tvl += vault.asset().balanceOf(address(vault)) 
                * vault.assetPrice() * 1e18
                / (10**vault.asset().decimals()) 
                / (10**vault.oracle().decimals());
      }
@>    uint numerator   = tvl - dyad.totalSupply();
      uint denominator = kerosineDenominator.denominator();
      return numerator * 1e8 / denominator;
  }

Old vaults are not added to KerosineManager in deploy script. It only adds newly deployed vaults:

    KerosineManager kerosineManager = new KerosineManager();

    kerosineManager.add(address(ethVault));
    kerosineManager.add(address(wstEth));

    vaultManager.setKeroseneManager(kerosineManager);

Tools Used

Manual Review

It must use only DYAD supply that is minted in V2 excluding tokens from V1.

Assessed type

Other

#0 - c4-pre-sort

2024-04-28T19:33:50Z

JustDravee marked the issue as duplicate of #308

#1 - c4-pre-sort

2024-04-29T09:05:13Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-11T20:08:36Z

koolexcrypto marked the issue as satisfactory

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/main/src/core/VaultManagerV2.sol#L221-L226

Vulnerability details

Impact

There are 2 type of vaults user can deposit into: usual Vault and KerosineVault. Usual Vault is expected to contain at least 100% of CR, i.e. at most 50% of CR can be kept in KerosineVaults (MCR is 150%).

Problem is that on liquidation it loops only through usual Vaults, it misses KerosineVaults. As a result, liquidator loses money on liquidation.

Proof of Concept

Suppose following scenario:

  1. User deposited 1000 USD in usual Vault and 500 USD in KerosineVault, borrowed 1000 USD in DYAD.
  2. Suppose user is liquidateable with CR = 150%. Liquidation bonus is 20%.
  3. Liquidation is performed according to this code:
      uint cr = collatRatio(id);

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

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

liquidationAssetShare = ((1.5 - 1) * 0.2 + 1) / 1.5 = 0.73

As a result, liquidator will pay 1000 USD in DYAD and receive only 1000 USD * 0.73 = 730 USD.

Here you can see that it loops only through usual vaults: https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/VaultManagerV2.sol#L221-L226

Tools Used

Manual Review

Loop through Kerosine Vaults too.

Assessed type

Other

#0 - c4-pre-sort

2024-04-28T10:20:36Z

JustDravee marked the issue as duplicate of #128

#1 - c4-pre-sort

2024-04-29T09:08:07Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-11T19:42:44Z

koolexcrypto marked the issue as satisfactory

Awards

17.2908 USDC - $17.29

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
edited-by-warden
:robot:_75_group
duplicate-977

External Links

Lines of code

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

Vulnerability details

Impact

Liquidation bonus is defined as 20% on excessive collateral of position. MCR is defined as 150%, i.e. liquidator receives at max 10% bonus of position amount.

Problem is that in case user's CR rapidly falls under 100%, liquidation is unprofitable because liquidator repays DYAD debt which is greater than collateral

As a result, protocol accumulates bad debt and DYAD can depeg.

Proof of Concept

Here you can see that 20% bonus is applied only for excessive collateral above 100% CR. So in case CR is less than 100%, liquidator pays more than receives:

  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();
      for (uint i = 0; i < numberOfVaults; i++) {
          Vault vault      = Vault(vaults[id].at(i));
@>        uint  collateral = vault.id2asset(id).mulWadUp(liquidationAssetShare);
          vault.move(id, to, collateral);
      }
      emit Liquidate(id, msg.sender, to);
  }

Finally if bad debt accumulates, TVL can become less than DYAD totalSupply. It will make all the withdrawals and liquidations revert because UnboundedKerosineVault.assetPrice() will underflow:

  function assetPrice() 
    public 
    view 
    override
    returns (uint) {
      uint tvl;
      address[] memory vaults = kerosineManager.getVaults();
      uint numberOfVaults = vaults.length;
      for (uint i = 0; i < numberOfVaults; i++) {
        Vault vault = Vault(vaults[i]);
        tvl += vault.asset().balanceOf(address(vault)) 
                * vault.assetPrice() * 1e18
                / (10**vault.asset().decimals()) 
                / (10**vault.oracle().decimals());
      }
@>    uint numerator   = tvl - dyad.totalSupply();
      ...
  }

Tools Used

Manual Review

Consider implementing mechanism for bad debt handling.

Assessed type

Other

#0 - c4-pre-sort

2024-04-28T10:10:40Z

JustDravee marked the issue as duplicate of #977

#1 - c4-pre-sort

2024-04-29T09:23:42Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-04T09:44:04Z

koolexcrypto changed the severity to QA (Quality Assurance)

#3 - c4-judge

2024-05-12T09:23:58Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#4 - c4-judge

2024-05-28T16:20:19Z

This previously downgraded issue has been upgraded by koolexcrypto

#5 - c4-judge

2024-05-28T16:21:49Z

koolexcrypto marked the issue as satisfactory

Findings Information

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
edited-by-warden
:robot:_78_group
duplicate-829

Awards

122.4433 USDC - $122.44

External Links

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/4a987e536576139793a1c04690336d06c93fca90/script/deploy/Deploy.V2.s.sol#L35

Vulnerability details

Impact

BoundedKerosineVault has inner unboundedKerosineVault from which he fetches Kerosine price.

Problem during deploy is that function setUnboundedKerosineVault() is not called. BoundedKerosineVault is meant to be added later. As a result there will be a problem if bounded vault is added as is: assetPrice() will revert and therefore core function VaultManagerV2.getKeroseneValueI() will revert. It will cause withdrawals and liquidations revert.

Proof of Concept

Here you can see that BoundedKerosineVault fetches price from UnboundedKerosineVault:

  function setUnboundedKerosineVault(
    UnboundedKerosineVault _unboundedKerosineVault
  )
    external
    onlyOwner
  {
    unboundedKerosineVault = _unboundedKerosineVault;
  }

  function assetPrice() 
    public 
    view 
    override
    returns (uint) {
      return unboundedKerosineVault.assetPrice() * 2;
  }

However it doesn't call BoundedKerosineVault.setUnboundedKerosineVault() in deploy script.

Tools Used

Manual Review

Set address in bounded vault:

    BoundedKerosineVault boundedKerosineVault     = new BoundedKerosineVault(
      vaultManager,
      Kerosine(MAINNET_KEROSENE), 
      kerosineManager
    );
+   boundedKerosineVault.setUnboundedKerosineVault(unboundedKerosineVault);

Assessed type

Other

#0 - c4-pre-sort

2024-04-29T07:37:31Z

JustDravee marked the issue as duplicate of #829

#1 - c4-pre-sort

2024-04-29T09:22:42Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-05T10:52:12Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#3 - c4-judge

2024-05-29T12:33:29Z

koolexcrypto marked the issue as satisfactory

Awards

4.8719 USDC - $4.87

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
:robot:_75_group
duplicate-175

External Links

Lines of code

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

Vulnerability details

Impact

Liquidator bonus is defined as 20% on excessive collateral above 100% CR. This 20% bonus is measured in positions amount. Problem is that Liquidator is not incentivized to liquidate positions where liquidation bonus is less than gas costs for performing liquidation.

Proof of Concept

Here you can see that 20% liquidation bonus is applied on position size:

  function liquidate(
    uint id,
    uint to
  ) 
    external 
      isValidDNft(id)
      isValidDNft(to)
    {
      ...

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

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

Tools Used

Manual Review

Require min amount to be left on balance. Or introduce has stipend: fixed amount like 0.2 ETH which is deposited once and can be claimed by liquidator.

Assessed type

Other

#0 - c4-pre-sort

2024-04-29T07:52:27Z

JustDravee marked the issue as duplicate of #1258

#1 - c4-pre-sort

2024-04-29T09:16:29Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-03T14:07:47Z

koolexcrypto changed the severity to QA (Quality Assurance)

#3 - c4-judge

2024-05-22T14:26:07Z

This previously downgraded issue has been upgraded by koolexcrypto

#4 - c4-judge

2024-05-28T16:53:14Z

koolexcrypto marked the issue as satisfactory

#5 - c4-judge

2024-05-28T20:05:53Z

koolexcrypto marked the issue as duplicate of #175

Findings Information

🌟 Selected for report: Pataroff

Also found by: Egis_Security, Evo, Jorgect, MrPotatoMagic, SBSecurity, T1MOH, carrotsmuggler, ljj

Labels

bug
2 (Med Risk)
satisfactory
sufficient quality report
:robot:_74_group
duplicate-100

Awards

223.9474 USDC - $223.95

External Links

Lines of code

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

Vulnerability details

Impact

VaultManagerV2 allows to burn DYAD for other position, it only checks that position exists.

Also when user redeems DYAD, it will revert on underflow in case user redeems more DYAD than his debt is. Combining these things attacker can frontrun users who redeem full debt by repaying 1 wei - users' transactions will fail.

Proof of Concept

Here you can see that user can repay debt on behalf of another valid position and it will underflow in dyad.burn():

  function burnDyad(
    uint id,
    uint amount
  ) 
    external 
@>    isValidDNft(id)
  {
    dyad.burn(id, msg.sender, amount);
    emit BurnDyad(id, amount, msg.sender);
  }

  function burn(
      uint    id, 
      address from,
      uint    amount
  ) external 
      licensedVaultManager 
    {
      _burn(from, amount);
@>    mintedDyad[msg.sender][id] -= amount;
  }

  function redeemDyad(
    uint    id,
    address vault,
    uint    amount,
    address to
  )
    external 
      isDNftOwner(id)
    returns (uint) { 
@>    dyad.burn(id, msg.sender, amount);
      ...
  }

Tools Used

Manual Review

There are 2 options:

  1. Use isDNftOwner(id) in function burnDyad().
  2. Refactor redeemDyad() to not underflow if specified amount to repay is greater than actual debt.

Assessed type

DoS

#0 - c4-pre-sort

2024-04-28T05:31:38Z

JustDravee marked the issue as duplicate of #74

#1 - c4-pre-sort

2024-04-29T11:58:16Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-10T10:14:46Z

koolexcrypto marked the issue as duplicate of #992

#3 - c4-judge

2024-05-11T12:15:58Z

koolexcrypto marked the issue as satisfactory

#4 - c4-judge

2024-05-28T10:29:41Z

koolexcrypto marked the issue as duplicate of #100

Awards

4.8719 USDC - $4.87

Labels

bug
2 (Med Risk)
downgraded by judge
satisfactory
sufficient quality report
edited-by-warden
:robot:_67_group
duplicate-67

External Links

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/main/src/core/Vault.kerosine.unbounded.sol#L50-L68

Vulnerability details

Impact

Firstly need to list formulas.

Kerosine price is determined as $Kp = \frac{TVL - DyadTotalSupply}{KerosineTotalSupply}$

Collateral Ratio is calculated as $CR = \frac{exogenousCollateralUSD + Kp * kerosineDeposit}{borrowedDyad}$

Here is attack idea with hypothetical numbers:

  1. Suppose 1 DAI is 1e18 and 1 USD for simplicity and Dyad has 18 decimals. V2 is deployed recently and has 0 TVL. Let's say Kerosine supply is 1e24 in circulation.
  2. Attacker is Whale, he deposits 100e24 DAI (100M USD).
  3. User deposits 1.4e24 DAI (1.4M USD) with 0.0025e24 of Kerosine and borrows 1e24 of DYAD. Now Kerosine price is $Kp = \frac{101.4e24 - 1e24}{1e24} = 100.4$. User's collRatio is $CR = \frac{1.4e24 + 0.0025e24 * 100.4}{1e24} = 1.651$. Note CR is high enough, MCR = 1.5
  4. Now attacker mints DYAD against his deposit. At max he can mint $\frac{100e24}{1.5} = 66.6e24$ DYAD
  5. Now Kerosine price is $Kp = \frac{101.4e24 - (1e24 + 66.6e24)}{1e24} = 33.8$. It felt almost 3 times down. Now user's collRatio is $CR = \frac{1.4e24 + 0.0025e24 * 33.8}{1e24} = 1.48$. It means user is subject to liquidation, so attacker can liquidate him and receive $1e24 * \frac{(1.48 - 1) * 0.2 + 1}{1.48} = 0.74e24$ DAI in return. From this single user attacker's profit is 0.72% of initial deposit
  6. Attacker burns DYAD minted in step 4 and is ready to sandwich new users.

What is the core of manipulation? Whale can decrease Kerosine price by minting DYAD against his deposit: it increases DYAD supply making Kerosine price as low as third of previous value, it is because he can mint up to 2/3 of his deposited amount. And because it is whale, his deposited amount is close to TVL, so now we get different formula for Kerosine price at the end of manipulation: $X = \frac{WhaleDeposit - 2/3 * WhaleDeposit}{KerosinTotalSupply} = 1/3 * \frac{WhaleDeposit}{KerosinTotalSupply}$

Proof of Concept

Here is test with scenario described above. Profit in step 5 slightly differs, didn't investigated much. In test it says 1.01e24 instead of theoretical 0.72e24.

// SPDX-License-Identifier: MIT
pragma solidity =0.8.17;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import {DeployBase, Contracts} from "../script/deploy/DeployBase.s.sol";
import {Parameters} from "../src/params/Parameters.sol";
import {DNft} from "../src/core/DNft.sol";
import {Dyad} from "../src/core/Dyad.sol";
import {Licenser} from "../src/core/Licenser.sol";
import {VaultManager} from "../src/core/VaultManager.sol";
import {Vault} from "../src/core/Vault.sol";
import {Payments} from "../src/periphery/Payments.sol";
import {OracleMock} from "./OracleMock.sol";
import {ERC20Mock} from "./ERC20Mock.sol";
import {IAggregatorV3} from "../src/interfaces/IAggregatorV3.sol";
import {ERC20} from "@solmate/src/tokens/ERC20.sol";
import "../src/staking/Kerosine.sol";
import "../src/core/KerosineManager.sol";
import "../src/core/VaultManagerV2.sol";
import "../src/core/Vault.kerosine.unbounded.sol";

contract CustomTest is Test {

    address user = address(1);
    address whale = address(2);

    Licenser licenser;
    Dyad dyad;
    Kerosine kerosine;
    KerosineManager kerosineManager;
    ERC20Mock usdc;
    Licenser vaultLicenser;
    DNft dnft;
    OracleMock usdcOracle;
    VaultManagerV2 vaultManagerV2;
    Vault usdcVault;
    UnboundedKerosineVault unboundedKerosineVault;
    KerosineDenominator kerosineDenominator;
    uint256 whaleId;
    uint256 userId;

    function setUp() public {
        // 1. Deploy

        licenser = new Licenser();
        dyad = new Dyad(licenser);
        kerosine = Kerosine(address(new ERC20Mock("", "")));
        kerosineManager = new KerosineManager();
        usdc = new ERC20Mock("", "");
        vaultLicenser = new Licenser();
        dnft = new DNft();
        usdcOracle = new OracleMock(1e8);

        vaultManagerV2 = new VaultManagerV2(
            dnft,
            dyad,
            vaultLicenser
        );

        licenser.add(address(vaultManagerV2));

        usdcVault = new Vault(
            vaultManagerV2,
            usdc,
            IAggregatorV3(address(usdcOracle))
        );

        kerosineManager.add(address(usdcVault));

        vaultManagerV2.setKeroseneManager(kerosineManager);
        
        unboundedKerosineVault = new UnboundedKerosineVault(
            vaultManagerV2,
            kerosine, 
            dyad,
            kerosineManager
        );
        kerosineDenominator = new KerosineDenominator(
            kerosine
        );
        unboundedKerosineVault.setDenominator(kerosineDenominator);

        vaultLicenser.add(address(usdcVault));
        vaultLicenser.add(address(unboundedKerosineVault));

        // 2. Set initial values

        // set Kerosine totalSupply
        ERC20Mock(payable(address(kerosine))).mint(user, 1e24);

        // mint USDC and DNft
        usdc.mint(whale, 100e24);
        usdc.mint(user, 1.4e24);
        whaleId = dnft.mintNft{value: 1 ether}(whale);
        userId = dnft.mintNft{value: 1 ether}(user);
    }

    function test_custom() public {
        // Step 2

        vm.startPrank(whale);
        usdc.approve(address(vaultManagerV2), 100e24);
        vaultManagerV2.add(whaleId, address(usdcVault));
        vaultManagerV2.deposit(whaleId, address(usdcVault), 100e24);
        vm.stopPrank();

        // Step 3

        vm.startPrank(user);
        usdc.approve(address(vaultManagerV2), 1.4e24);
        vaultManagerV2.add(userId, address(usdcVault));
        vaultManagerV2.deposit(userId, address(usdcVault), 1.4e24);

        kerosine.approve(address(vaultManagerV2), 0.0025e24);
        vaultManagerV2.add(userId, address(unboundedKerosineVault));
        vaultManagerV2.deposit(userId, address(unboundedKerosineVault), 0.0025e24);

        vaultManagerV2.mintDyad(userId, 1e24, user);
        vm.stopPrank();

        console.log("User's collRatio after borrow: %e", vaultManagerV2.collatRatio(userId));

        // Step 4
        vm.prank(whale);
        vaultManagerV2.mintDyad(whaleId, 66.6e24, whale);

        // Step 5
        vm.prank(whale);
        vaultManagerV2.liquidate(userId, whaleId);


        // Step 6
        vm.startPrank(whale);
        vaultManagerV2.burnDyad(whaleId, dyad.balanceOf(whale));
        
        uint256 usdcWhaleBalance = usdcVault.id2asset(whaleId);
        console.log("USDC balance of Whale: %e", usdcWhaleBalance);
        console.log("Whale's profit: %e", usdcWhaleBalance - 100e24);
    }
}

Tools Used

Manual Review

I don't see easy mitigation to the issue, it can be design issue of Kerosine feature. Economical audit is heavily advised to ensure Kerosin feature doesn't introduce another flaws in design.

Assessed type

Other

#0 - c4-pre-sort

2024-04-28T06:10:59Z

JustDravee marked the issue as duplicate of #67

#1 - c4-pre-sort

2024-04-29T09:05:59Z

JustDravee marked the issue as sufficient quality report

#2 - c4-judge

2024-05-05T09:59:11Z

koolexcrypto changed the severity to 2 (Med Risk)

#3 - c4-judge

2024-05-08T11:50:06Z

koolexcrypto marked the issue as unsatisfactory: Invalid

#4 - c4-judge

2024-05-08T12:57:50Z

koolexcrypto marked the issue as nullified

#5 - c4-judge

2024-05-08T12:57:54Z

koolexcrypto marked the issue as not nullified

#6 - c4-judge

2024-05-11T19:25:39Z

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