A Hardcoded Health Factor That Liquidates Healthy DeFi Positions

This was my first CodeHawks contest. The platform was young, I was newer to competitive auditing than I'd like to admit, and the target was the Cyfrin "Foundry DeFi Stablecoin" system that a lot of people first meet through the Updraft course. So there's a bit of nostalgia attached to this one. It's also a clean example of a bug class I still flag on every DeFi review I do: a contract that hardcodes a decimal assumption and then advertises that it works with tokens that break it.

The system is an over-collateralized stablecoin. Two contracts do the work. DecentralizedStableCoin is the ERC20 itself, a fairly boring mint-and-burn token. DSCEngine is the interesting part: it holds the collateral, tracks how much DSC each user has minted against it, and decides who is solvent and who is liquidatable. The pitch is the usual one. Deposit collateral worth more than the stablecoin you mint, keep your position above a health factor, and if you fall below it someone can liquidate you and pocket a bonus. The whole thing only stays pegged if "below the health factor" actually means "this position is undercollateralized." That sentence is where the contest lived for me.

The finding

The engine measures solvency with a health factor and compares it against a single constant:

uint256 private constant LIQUIDATION_THRESHOLD = 50; // 200% overcollateralized
uint256 private constant LIQUIDATION_PRECISION = 100;
uint256 private constant MIN_HEALTH_FACTOR = 1e18;
uint256 private constant PRECISION = 1e18;

MIN_HEALTH_FACTOR is hardcoded to 1e18. Anything under that and you're fair game:

function _revertIfHealthFactorIsBroken(address user) internal view {
    uint256 userHealthFactor = _healthFactor(user);
    if (userHealthFactor < MIN_HEALTH_FACTOR) {
        revert DSCEngine__BreaksHealthFactor(userHealthFactor);
    }
}

That 1e18 only makes sense if every number feeding the health factor is also scaled to 18 decimals. Follow where the collateral value comes from and you find the assumption baked in even deeper:

function getUsdValue(address token, uint256 amount) public view returns (uint256) {
    AggregatorV3Interface priceFeed = AggregatorV3Interface(s_priceFeeds[token]);
    (, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
    return ((uint256(price) * ADDITIONAL_FEED_PRECISION) * amount) / PRECISION;
}

amount here is the raw token balance, in the token's own decimals. Divide by PRECISION, which is 1e18, and the math is correct for an 18-decimal token like WETH or WBTC. One WETH comes in as 1e18, the division cancels it, and you get a clean USD figure. The health factor that uses this value then compares against 1e18 and everything lines up.

The problem is the contract doesn't only support WETH and WBTC. When I went digging into what tokens this engine was actually meant to handle, the answer wasn't in the code, it was in the sponsor's own words in the contest Discord: it's supposed to work with any Chainlink-supported token. Plenty of those don't use 18 decimals. USDC and USDT use six. So the engine claims one thing in conversation and assumes another in its arithmetic.

Walk a six-decimal token through getUsdValue. One USDC arrives as 1e6, not 1e18. The function still divides by 1e18, so the USD value it computes is off by a factor of 1e12. The collateral isn't slightly mispriced, it's effectively rounded down to nothing. That crushed value flows straight into _healthFactor, the health factor lands far below 1e18, and a position that is genuinely sitting at 200% collateralization reads as insolvent to the contract.

How I found it

Honestly, not by being clever. By being slow, which is the only trick I actually trust. I traced the liquidation path backwards. I started at "who can be liquidated," which is _revertIfHealthFactorIsBroken and its MIN_HEALTH_FACTOR < 1e18 check, and then I kept asking where each number came from until I hit a token amount. That walk ends at getUsdValue, and the moment you see a raw amount getting divided by a hardcoded 1e18, the question writes itself: what happens when amount isn't 18 decimals?

The code on its own never answers that, because in the test suite the collateral is always WETH or WBTC and the assumption holds perfectly. This is exactly why I read the contest page and the sponsor channel before I trust the code, and it's a habit I lean on so hard I wrote a whole piece about how I actually audit a contract. The vulnerability didn't exist inside any single function. It existed in the gap between what the contract assumed about its inputs and what the sponsor said the inputs could be. You only see that gap if you go find the spec that isn't in the repo.

Once I had that, the second high severity finding fell out of the same root. The decimal flaw in getUsdValue mis-prices collateral on redemption too, so users pull out less than they're owed. Same hardcoded assumption, two different ways for it to hurt people.

Why it threatens solvency

This is worse than a rounding nuisance, and it's worth being precise about why.

The point of a liquidation mechanism is to protect the peg by removing positions that no longer back their debt. It is supposed to fire on undercollateralized users and leave healthy ones alone. Here the trigger is wired to a distorted measurement. A user can deposit a six-decimal token, mint well within the limit, sit at double the collateral they need, and still present a health factor below 1e18. The contract doesn't see a healthy position. It sees an insolvent one and opens the door to liquidation.

So someone deposits real value, does nothing wrong, stays comfortably over-collateralized by the protocol's own stated rules, and can be liquidated the moment they mint. Their collateral gets sold out from under them, the liquidator collects the bonus, and the loss is just gone. The bigger the position, the bigger the hit, and there's no attack setup required. It's the normal happy path misfiring on a token the protocol claims to support. That combination, certain to trigger and directly destroying user funds, is why I rated it High.

There's a second-order problem too. A liquidation engine that fires on solvent positions is also one you can't trust to fire correctly on insolvent ones, because the number it's reading isn't measuring what it thinks it's measuring. Once the solvency signal is wrong, every decision built on top of it is suspect, peg defense included.

The fix

The recommendation I gave was to stop hardcoding the decimal assumption and build a liquidation path that only liquidates positions that are genuinely insolvent. Concretely that means normalizing every collateral token to a common scale before it touches the health factor, instead of assuming 18 decimals and dividing by a flat 1e18. Chainlink feeds expose decimals(), and ERC20s expose their own; you read both and scale deliberately rather than hoping every token matches WETH.

I deliberately didn't hand over a one-line patch dressed up as a complete fix, because it isn't one. The hardcoded 1e18 is a symptom. The actual defect is that solvency math runs on un-normalized amounts, and that assumption is spread across getUsdValue, the health factor calculation, and the liquidation check. You fix it by deciding on a single internal precision and converting everything into it at the boundary, once, so the rest of the engine can stop guessing. Patch only the constant and you've moved the bug, not killed it.

The lesson

The thing I took from this contest, and still carry, is that a hardcoded constant is a claim about every input the code will ever see. 1e18 isn't just a number, it's the contract asserting "all my collateral is 18 decimals." The instant the protocol also says "I work with any Chainlink token," those two statements contradict each other, and the contradiction is the bug. Nothing in the Solidity is wrong on its own line. The defect lives in the distance between an assumption and a promise.

That's also why the boring reconnaissance keeps earning its place. The detail that turned this from a theoretical "what if the token isn't 18 decimals" into a real, rateable finding was a sentence in the sponsor's Discord, not anything in the source. If I'd only read the code, I'd have had a shrug. Reading the scope and the sponsor's own description of intent is what gave the finding teeth.

If you're shipping a protocol that takes more than one collateral type, this is precisely the kind of seam worth having read by someone who follows the numbers instead of skimming the functions. That's the work I do.