# Dyad Review Log

Dyad is a stablecoin creation protocol which uses lending against modified 4626 vault deposits to create stablecoins. The scope of the audit is around 800 lines of solidity which includes the ERC4626 contracts, NFT contracts, Governance system and liquidation system, but not the staking system. The lending protocol uses a simple constant 150% over collateralization rule and has permissionless collateral addition system. The review was conducted over a week starting with commit 6c49aca.

Note: Normally issues in the log are marked as resolved, acknowledged or disputed but after this review the client has decided to largely depreciate/rework the codebase and so I am not tracking the remediations for each issue.

## Issues

Summary: Critical: 7, High: 4, Medium 0, Low: 0, Note: Various

Presented in chronological order.

* Sybil attack on majority vote system allows infinite mint of dyad stablecoin: The stablecoin Dyad is minted by contracts which are called vaults and those vaults are registered inside of a voting system called the SLL contract. The SLL contract allows any NFT to vote once and if a percentage agreement threshold is passed then among all NFTs which have voted then a vault can be added to an approved mapping in the SLL contract. Each NFT can be minted for a price equal to base price + increase * totalSupply, there are no limits on total mint or rate limits. The Dyad stablecoin contains mintTo and burnFrom methods which check that the msg.sender is in the approved mapping of the SLL contract.
If an adversary pays a cost $c = 2t(b + 2ti)%$ (where t is the starting total supply, b is base price, and i is increase) then they can purchase a super majority and vote in any address as a vault, including an EOA. The EOA can then call the mint function to mint infinite Dyad, or the burn function to burn Dyad from any address. Allowing them to drain any onchain AMM pools for DYAD, and also to lock all collateral into the system by burning dyad from every address other than their own so no repayments can function. They can then wait for liquidations to happen and liquidate other users. Because the liquidation process transfers the note's debt and collateral to the liquidator, the EOA can then mint themselves dyad and withdraw the other user's collateral.
* Severity Estimate: Critical [Likihood: High, Impact: High]
* Remediation Estimate: The team's intended goal of permissionless collateral addition is very hard to achieve in a safe way. At a minimum the team should be using a factory or create2 system to ensure that the vaults which are voted on are in fact vault contracts and also a delay system to ensure collateral types and new vaults cannot be added then immediately used. Even this is likely to have other issues as the voting is still strongly sybilable.
* Bonus Sent to Liquidated Account Not the Liquidator: the bonus payouts for liquidators first payout by transfering the liquidated person's shares to the liquidator but then mint the bonus shares to the liquidated party instead of the liquidator. This can make payouts less profitable than intended.
* Severity Estimate: High [Impact: Medium, Likelihood: High]
* Remediation Recommendation: Change "from" to "to" in the mint call.
* Liquidation payouts double the value of the collateral: When liquidating an account a bonus calculation is done which checks how much less than the 150% collateralization threshold and then adds a bonus by minting dilutive shares in the 4626 vaults. The calculation is preformed as (debt/collateral - .66) = bonus ratio, then a call is made to mint which mints (1+bonus ratio)\*shares, this should be (bonus ratio)\*shares. The current version doubles the collateral value whenever an account is liquidated as the shares owned by the nft account and the new minted shares are both sent to the liquidator. This will cause the protocol to rapidly tend to insolvency when there are liquidations, and in some circumstances MEV bundles can be used to drain the protocol by liquidating the attackers own account.
* Severity Estimate: Critical [Impact: High, Likelihood: High]
* Remediation Recommendation: Remove the 1e18 from the bonus calculation.
* De-listed assets loans cannot be liquidated: The bonus ratio is calculated by dividing the debt by the USD value of the assets. The value should never be zero if using proper oracles, however when a vault is removed from the SLL the collateral value is set to zero, so if that is the only collateral the loan cannot be liquidated because the division by zero reverts.
* Severity Estimate: High [Impact: Medium, Likelihood: High]
* Remediation Recommendation: Special case zero value accounts
* Bonus calculation allows draining a vault when it is being voted out: The bonus payout is calculated as (debt/assets - 0.66)\*shares. So supposed that a vault is being removed with a collateral which is expected to retain value, such as migrating to a new USDC vault version. In this case because each vote is one off we can expect that the vault is highly likely to reach a position where between 1 and 100 votes can change a vault from approved to unapproved. A user can deposit into the collateral vault which is almost unapproved and into a second account with a very small amount. Say there is 1 million in the USDC vault and a malicious user deposits 150 usdc and 0.000001 eth then takes a 100 dyad loan. They then use a MEV bundle or smart contract call to place the final vote for removal of the usdc vault and then liquidate their own loan. Their liquidation bonus is then calculated as (100/0.000001 - 0.66)\*150 and they are minted 14,999,999,901 shares of the usdc vault. They can then withdraw and take out approximately 999,999 usdc from the vault. It is possible to also preform this attack by buying up or minting enough of the NFTs and then voting out any vault you want. It's fairly likely if the system is widely used this will happen completely on accident.
* Severity Estimate: Critical [Impact:High, Likelihood: High]
* Remediation Recommendation: It's hard to remediate every possible case of this while preserving the bonus mechanism, however the case of triggering this via voting can be mediated by reducing the value of collateral over time when it is voted out. One major consideration left out by this fix is that sometimes collateral value naturally goes to zero (in hacks of the the collateral's source protocol for instance), in this case you can see accidental triggers of this case.
* Liquidation cascades because of the bonus dilution mechanic: When a liquidation occurs the bonus mechanism triggers and mints a liquidator enough of the shares to make it so that they always recover 150% of the debt repaid. However by minting those new shares it reduces the value of the collateral of other loans and all other loans are backed by 4626 vaults. Depending on the configuration of the outstanding loans and balances this can lead to loans being liquidated by price movements which would not normally liquidate them (which can then liquidate more loans and so on). Liquidation cascades are possible but only in rare distributions of loans. However if the prices of assets are going down this produces a super linear downward price effect on collateral, as the price reducing reduces the numerator of the share price and the dilutative effect of liquidations increases the denominator. This can lead to very rapid unwinds of all loans in the system, and the multi-collateralization system can cause unwinds in one asset to spread to multiple assets. We note: that this version of the mechanic does not match how the client or the docs have described the liquidation system.
* Severity Estimate: High [Impact: High, Likelihood: Medium]
* Remediation Recommendation: the bonus system code should be changed to match the descriptions of the client and the docs.
* Insolvency or incorrect collateral values with some oracles: The VaultManager contract calls the vault which calls chainlink to load the USD value of the assets. Chainlink's oracle has a decimals field which is the decimals of their quote's fixed point encoding, critically it is not the decimals of the underlying asset. When the VaultManager calls `vault.decimals()` it calls the decimals method of the ERC20 which is inherited by the solmate implementation of 4626, this method returns the fixed point decimals of the underlying asset. Therefore when the VaultManager calculates the USD value of assets as
`vault.convertToAssets(shares) * vault.collatPrice()/vault.decimals()` it will produce an incorrect answer when the chainlink price quote does not match the decimals of the underlying asset. In the two cases of the MKR and DAI vs USD feeds they are both 18 point which will generate incorrect values in the redeem function when the redeemed share value is calculated as: `amount * (10**_vault.decimals()) / _vault.collatPrice();` which will allow the user to withdraw 10^10 more collateral from the eth vault than is intended. In the opposite case the USDC/USD price feed has decimals 10^2 higher than the asset and so you will be able to create 100x more debt than intended from USDC vaults.
* Severity Estimate: Critical [Likelihood: High, Impact: High]
* Remediation Recommendation: Use the decimals from the price feed not the vault to calculate the value.
* Infinite debt creation: The function `mintDyad(uint from, address to, uint amount)` allows a user to create new dyad loans. The checks that it enforces are that you own the NFT 'from' and that `collatRatio(from) < MIN_COLLATERIZATION_RATIO`. However the collateralization ratio is loaded from your current loan positions, and so will not reject if you take on more debt than your collateral is worth. This means you can call the function with amount = uint256.max and mint uint256 max dyad to yourself, allowing you to instantly drain the liquidity pools for dyad.
* Severity Estimate: Critical [Likelihood: High, Impact: High]
* Remediation Recommendation: Calculate the user's collateralization ratio after their requested debt not before.
* Infinite Dyad mints from debt cycling: Even assuming the previous bug was fixed a user could an unlimited amount of bad debt by: depositing into a vault, minting dyad, withdrawing directly from the vault, and then repeating from a new NFT. The NFT in question is instantly put into a liquidatable state, but will contain no assets. Given this the user can pay the cost to mint the NFTs with enough mints can then fully drain the liquidity pools for dyad by creating hundreds of bad loans. Or for vault assets which can be flash loaned the user can flash loan in a very large amount of capital, draw a loan against it, and then withdraw it from the vault to repay the flashloan.
This is also a vector for a variant of the attack where the bonus calculation is used to drain vaults: in this variant the user deposits into two vaults one which will be drained and one which will create the bad debt. For example supposed attacker then deposits 1 usd of capital into the vault they wish to drain and 150 usd into the vault they will withdraw from. They then draw 100 usd of debt, withdraw the 150 of backing and liquidate themselves; because the bonus mechanic will multiply their shares in the drained vault by 100/1 = 100x, and then withdraw 101 usdc. This can be used to empty the protocol of all collateral and also the liquidity pools of all value.
* Severity Estimate: Critical [Likelihood: High, Impact: High]
* Remediation Recommendation: The vaults should call into the vault manager and check that the user is not creating bad debt before allowing direct withdraws.
* Vault removal of vaults which contain the collateral for live loans allows theft of collateral or prevention of liquidations: The `remove` function does not check if the vault the user is removing from their list of backing accounts is currently in use backing live loans. This has a number of interesting effects. As has been discussed in two previous classes of bugs the bonus shares mechanic can allow theft of collateral assets if there is a discontinuity/zeroing in collateral value backing a loan. In this case we can use the remove function to access this class of bug by: depositing 150 USDC and 1 DAI to two vaults, borrowing 100 dyad, calling remove to remove the USDC vault from the account's list of backings, then liquidating the account to generate 100/1 x 1 = 100 dai shares, then by calling add again we can restore our access to our 150 usdc and profit 100 dai.
Allowing removals of vaults which currently have capital does not just allow access of the bonus mechanic bug, it also allows us to generate un-liqudiatable loans: suppose a user A does not wish to be liqudiated on a portfolio of 150 ETH and 150 bitcoin that they have drawn 200 dyad against, they can just call remove on both vaults and if a liquidator does call the liqudiation function they will not get any share or any bonus as both the shares an bonus are paid out in a for loop over the user's vaults array which in this case will be zero length.
* Severity Estimate: Critical [Likelihood: High, Impact: High]
* Remediation Recommendation: Vaults should be marked as in use and not be removable when they are in use.
* ERC4626 Shares are Priced Incorrectly in Redeem: The redeem function burns dyad and then withdraws ERC4626 shares to cover this amount. It makes the assumption that each ERC4626 share is worth exactly one of the asset in question, this is not true in general as the value of each share can go up when interest is paid to the 4626 vault or down when as a part of liquidations new shares are minted. Either case causes the shares which are withdrawn from the user's nft's 4626 account to not match the redeemed value. The impact of this depends highly on the previous bug's remediation as currently the collateral is not held ad so there is no reason to call redeem but also in the redeem function it only releases vault shares equal to exactly the value of the dyad (under the flawed calculation) and so it's not clear how the remaining 50% of collateral would be handled in a post remediation world. However post that code fix this bug could either result in a user being able to repay too little to remove their debt (in the case of share deflation) or being unable to repay it if the price per share were set very high.
* Severity Estimate: High [Likelihood: High, Impact: Medium]
### Notes:

* It's slightly odd to have a rate limited insider mint which is free but then also have all funds claimed by the admin as well. The admin can bypass the free mint by just using the public mint and collecting funds, however this will increase the future price of NFTs for other users.
*