Exploring Missed Vulnerabilities: CodeHawks Foundry DeFi Stablecoin Audit Contest

Prior to delving into the results summary, I strongly recommend watching Patrick Collins comprehensive 5-hour tutorial on building the project from scratch, which can be found on YouTube. This project’s objective is the creation of a decentralized stable coin backed by a basket of assets determined by the project owner, such as WETH and WBTC.

Security auditors have identified a total of 4 high-risk, 12 medium-risk, and 7 low-risk issues, along with 45 gas/info-based findings. My primary focus will be on the Medium and Low-risk categories, although I will also highlight noteworthy Gas/Info issues. I will explain the actual vulnerability and share what I have learnt with the goal to avoid similar mistakes in the future.

The Findings

H-01. Theft of collateral tokens with fewer than 18 decimals

The Vulnerability: The token prices computed by DSCEngine#getTokenAmountFromUsd() and DSCEngine#getUsdValue() fail to account for token decimals. These methods assume that all tokens have 18 decimals but token like WBTC has only 8 decimals.

Learning: While many tokens adhere to the standard of 18 decimals, exceptions exist, such as USDT/USDC (6 decimals) and WBTC (8 decimals). It’s crucial to check whether decimals are hardcoded or retrieved through the token’s interface to ensure accurate calculations.

H-02. Liquidation Is Prevented Due To Strict Implementation of Liqudation Bonus

The Vulnerability: A normal user should always be 200% over-collaterlized. but when a user is between 100% to 110% over-collateralized, they cannot be liquidated. The vulnerability allows users to avoid complete liquidation. The root cause of the issue is hardcoding 10% of the amount of liquidation to be sent to the liquidator as the liquidation bonus, this liquidation cannot be paid when a user is between 100% to 110% over-collateralized, hence the liquidation function will revert.

Learning: Always check for invariants whenever you see hardcoded value for rewards in the code like 10% liquidation bonus in this case.

H-03. There is no incentive to liquidate small positions

The Vulnerability: There is no incentive to liquidate low value accounts such as 5$ USD value accounts because of gas cost. The liquidator will end up paying more gas fees.

Learning: It’s crucial to factor in gas prices involving user rewards, such as liquidation, redeeming, or claiming rewards. This consideration ensures that users are appropriately incentivized, even for smaller positions.

H-04. Business Logic: Protocol Liquidation Arithmetic

The Vulnerability: To liquidate a users position in order to save the protocol from holding bad debt, the liquidator needs to pay back the DSC owed by the user that has a position at risk. Currently the only way to mint the DSC token is via the contract. But the math does not work out if user tries to mint new DSC tokens. Resulting in liquidators would not call liquidate.

Learning: This issue is more related to the design of the protocol. It basically makes it near impossible to liquidate any under collateralized positions without being the newest user of the protocol.

M-01. staleCheckLatestRoundData() does not check the status of the Arbitrum sequencer in Chainlink feeds.

The Vulnerability: a critical step that involves confirming the active status of the sequencer before trusting the data returned by the oracle in case of L2 like Arbitrum is missing. In the event of an Arbitrum Sequencer outage, the oracle data may become outdated, potentially leading to staleness.

M-02. DSC protocol can consume stale price data or cannot operate on some EVM chains

The Vulnerability: The stale period (3 hours) is too large for Ethereum, Polygon, BNB, and Optimism chains, leading to consuming stale price data. On the other hand, that period is too small for Arbitrum and Avalanche chains, rendering the DSC protocol unable to operate.

Learning: Both M-01 and M-02 share a common root cause. When deploying the contract on networks other than the Ethereum Mainnet, such as Arbitrum or Zksync, it is essential to thoroughly review the documentation, specifically focusing on the distinctions between these Layer 2 (L2) networks and the Ethereum Mainnet.

M-03. Chainlink oracle will return the wrong price if the aggregator hits minAnswer

The Vulnerability: Chainlink aggregators have a built-in circuit breaker if the price of an asset goes outside of a predetermined price band. The result is that if an asset experiences a huge drop in value (i.e. LUNA crash) the price of the oracle will continue to return the minPrice instead of the actual price of the asset and vice versa.

Learning: When dealing with Oracles, it’s crucial to verify whether the output of latestRoundData includes checks for minPrice.

M-04. All of the USD pair price feeds doesn’t have 8 decimals

The Vulnerability: DSCEngine contract assumes all of the USD pair chainlink price feeds have 8 decimals but there are certain token's USD feed has a different decimals. So if some tokens’s price feed has not 8 decimals it will break a lots of things in the contract.

Learning: This is a common issue. When protocols rely on external dependencies like Chainlink, it’s crucial to thoroughly review integration documents and past reported issues to avoid such assumptions. The trigger should be whenever you see a harcoded value.

M-05. Anyone can burn DecentralizedStableCoin tokens with burnFrom function

The Vulnerability: Anyone can burn DSC tokens with burnFrom function inherited of OZ ERC20Burnable contract because it does not have a onlyOwner modifier

Learning: Use the Solidity Metrics VS code extension by tintinweb to view all the functions visibility and access control.

M-06. Double-spending vulnerability leads to a disruption of the DSC token

The Vulnerability: The whitelisted collateral tokens will be registered along with their corresponding price feed addresses. However, the registration process does not verify that a token cannot be registered twice.

Learning: When working with arrays in Solidity, it’s crucial to implement checks to ensure the code handles situations like empty values and prevents duplicate entries. Developing a checklist for arrays can be helpful.

M-07. Lack of fallbacks for price feed oracle

The Vulnerability: In case Chainlink’s aggregators fail to update price data, the protocol will refuse to liquidate users’ positions, leading to the protocol’s disruption. The solution would be to use Uniswap TWAP price feed.

Learning: Always consider various scenarios and potential outcomes if the current external dependency in the code were to fail. Avoid assuming that it will work flawlessly 100% of the time.

M-08. Too many DSC tokens can get minted for fee-on-transfer tokens.

The Vulnerability: If one or more collateral tokens are fee-on-transfer tokens, i.e., when transferring X tokens, only X - F tokens arrive at the recipient side, where F denotes the transfer fee, depositors get credited too much collateral, meaning more DSC tokens can get minted, which leads to a potential depeg. Check the actual amount of received tokens instead of allowing users to specify.

Learning: Test the protocol’s compatibility with popular ERC20 tokens types like Fee-On-Transfer etc.,

M-09. liquidate does not allow the liquidator to liquidate a user if the liquidator HF < 1

The Vulnerability: Liquidators with a Health Factor (HF) below 1 are unable to initiate the liquidation of a user’s debt. This restriction prevents users from performing an action that doesn’t affect their own Health Factor.

Learning: This issue is rather unique. The lesson here is to assess whether critical functions can be accessed by all users.

M-10. Protocol can break for a token with a proxy and implementation contract (like TUSD)

The Vulnerability: For a token like TUSD , which has a proxy and implementation contract, if the implementation behind the proxy is changed, it can introduce features which break the protocol, like choosing to not return a bool on transfer(), or changing the balance over time like a rebasing token.

Learning: When working on a project, also take into account its compatibility with upgradable tokens.

M-11. Liquidators can be front-run to their loss

The Vulnerability: DSC liquidators are prone to oracle price manipulations and MEV front-run attacks. There is no slippage protection.

Learning: Whenever users tries to swap one token for another, always check if slippage protection is in place.

M-12. DoS of full liquidations are possible by frontrunning the liquidators

The Vulnerability: The current implementation requires liquidators to specify the amount of DSC tokens to burn. While partial liquidations function correctly, in the case of a full liquidation, a secondary user can execute a frontrunning attack by submitting just 1 wei of DSC tokens. This action causes the liquidator’s call to revert.

Learning: Look for Front-Running based attack vectors, especially for functions wherein value transfer is in place.

L-01. Improving the burnDsc() to allow users to mitigate their liquidation’s impact

The Vulnerability: In the situation that a user’s health factor is unhealthy (< MIN_HEALTH_FACTOR), the user will not be able to partially burn some available of their minted DSC tokens to mitigate the liquidation's impact.

Learning: When dealing with functions that involve value transfers, thoroughly assess both full and partial transfer scenarios.

L-05. Precision loss when calculating the health factor

The Vulnerability: the _calculateHealthFactor() suffers from a rounding down issue, resulting in a small precision loss that can be improved.

Learning: Always make sure the math equations follow multiplication before divide.

Gas Optimizations / Informationals

1. Using x=x+y /x=x-y is more gas efficient than x+=y / x-=y

2. burn() and staleCheckLatestRoundData() and getTimeout() can be external

3. ++i/i++ should be unchecked{++i}/unchecked{i++} when it is not possible for them to overflow, as is the case when used in for- and while-loops.

4. The nonReentrant modifier should occur before all other modifiers.

5. Not respecting the Checks-Effects-Interactions pattern that can be a place for bugs.

CodeHawks Foundry DeFi Stablecoin Audit Report — https://www.codehawks.com/report/cljx3b9390009liqwuedkn0m0

Thank For Reading.

Connect with me: https://linktr.ee/zuhaib44