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

8 stars 6 forks source link

Kerosine price can be negatively affected by current dispersal method #918

Closed c4-bot-8 closed 5 months ago

c4-bot-8 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/staking/KerosineDenominator.sol#L17 https://github.com/code-423n4/2024-04-dyad/blob/cd48c684a58158de444b24854ffd8f07d046c31b/src/core/Vault.kerosine.unbounded.sol#L65-L67

Vulnerability details

Relevant code: KerosineDenominator::denominator() UnboundedKerosineVault::assetPrice()

Description

Kerosine is a token designed to be used as additional collateral when minting DYAD. It's price is determined by a couple of factors: value of exogenous collateral, DYAD supply, and the Kerosine denominator.

The denominator itself is a value that currently is derived by subtracting Kerosine total supply (1B) from Kerosine in the DYAD multisig wallet (950M). At the time of this report, the denominator is ~50M.

Since Kerosine plays crucial role in maintaining a positions Collateral Ratio it is vital for all DYAD minters that this price is adjusted only via the mechanisms stated above. However with the current setup, minters who use Kerosine as collateral could fall below liquidation threshold.

In the docs it is stated:

There is a total supply of 1 billion Kerosene tokens emitted over 10 years. Users will earn Kerosene by providing liquidity for DYAD and staking the LP tokens.

Most of the Kerosine supply is in a multisig wallet and the mechanism for emitting Kerosene is not clear. There is no specific smart contract to be found in the repo. This means that as of now all positions using Kerosine for improving CR are in danger if the Kerosine in the multisig is transferred.

At first glance, the fact that UnboundedKerosineVault has a function setDenominator is a mitigating factor, because the Denominator contract can be changed. However to make this call and change the logic to something else, this new contract would first have to either own the Kerosine from the multisig (requiring transfer) or to have some temporary logic so CR doesn't fall.

In the first case CRs will fall because the bulk of the Kerosine was transferred. In the second case positions collateralized with Kerosine will not be liquidatable, opening up a massive arbitrage opportunity for traders.

Root Cause

Current method of calculating Kerosine price is affected by Kerosine moving out of the multisig wallet at any time or amount. Since these movements are unregulated, unexpected liquidations can occur for DYAD minters using Kerosine.

Impact

Kerosine price can change dramatically, causing liquidations to all who use it as part of the collateral.

Suggested Mitigation

Create a smart contract that owns the un-emitted Kerosine supply. Make a function emit() that distributes of the token and give the caller small Kerosine incentive. In this way, Kerosite emissions will be regular and risk for unexpected CR changes will be the lowest.

Assessed type

Other

c4-pre-sort commented 5 months ago

JustDravee marked the issue as duplicate of #67

c4-pre-sort commented 5 months ago

JustDravee marked the issue as not a duplicate

c4-pre-sort commented 5 months ago

JustDravee marked the issue as primary issue

c4-pre-sort commented 5 months ago

JustDravee marked the issue as duplicate of #673

c4-pre-sort commented 5 months ago

JustDravee marked the issue as sufficient quality report

c4-judge commented 5 months ago

koolexcrypto marked the issue as unsatisfactory: Invalid

yotov721 commented 4 months ago

Hi @koolexcrypto , This issue describes exact same mechanics and impact of #67 , thus being: Users using kerosine for collateral can be forcefully liquidated, because the dispersal of new kerosine tokens would lower their price hence user's collateral value. This is we we (team) think this issue is accurately describing the problem and proposing a solution thus is satisfactory.

koolexcrypto commented 4 months ago

Hi @yotov721

Thanks for the feedback.

should be duped with #67 with a partial credit since it doesn't specify how.

c4-judge commented 4 months ago

koolexcrypto removed the grade

c4-judge commented 4 months ago

koolexcrypto marked the issue as partial-75

c4-judge commented 4 months ago

koolexcrypto changed the severity to QA (Quality Assurance)