redeemAndBurn returns near-zero ETH and leaves collateral accounting stale

[syed-ghufran-hassan]

Description

The redemption flow in AAPL.redeemAndBurn is broken. It relies on getEthAmountFromUsd, which contains an extra * PRECISION in the denominator, causing the ETH payout to be effectively zero. Additionally, redeemAndBurn never subtracts the withdrawn ETH from s_ethCollateralPerUser[msg.sender], so collateral accounting becomes stale once the conversion bug is fixed.

Affected Code

function getEthAmountFromUsd(uint256 usdAmountInWei) public view returns (uint256) {
    AggregatorV3Interface priceFeed = AggregatorV3Interface(i_ethUsdFeed);
    (, int256 price,,,) = priceFeed.staleCheckLatestRoundData();
    return (usdAmountInWei * PRECISION) / ((uint256(price) * ADDITIONAL_FEED_PRECISION) * PRECISION);
}

function redeemAndBurn(uint256 amountToRedeem) external {
    uint256 valueRedeemed = getUsdAmountFromaapl(amountToRedeem);
    uint256 ethToReturn = getEthAmountFromUsd(valueRedeemed);
    s_aaplMintedPerUser[msg.sender] -= amountToRedeem;
    uint256 healthFactor = getHealthFactor(msg.sender);
    if (healthFactor < MIN_HEALTH_FACTOR) {
        revert AAPL_feeds__InsufficientCollateral();
    }
    _burn(msg.sender, amountToRedeem);

    (bool success,) = msg.sender.call{value: ethToReturn}("");
    if (!success) {
        revert("AAPL_feeds: transfer failed");
    }
}

Impact

• Users redeeming AAPL receive almost no ETH for their burned AAPL because getEthAmountFromUsd divides by an extra 1e18.

• The redeeming user’s ETH collateral balance is never reduced. After fixing the conversion bug, the contract would send ETH out while still counting that ETH as collateral, inflating health factors and potentially enabling over-minting or draining of protocol collateral.

Example

• Assume ETH = $3,000 and AAPL = $200. Redeem 1 AAPL:

• valueRedeemed = 200e18

• Correct ethToReturn ≈ 0.0666e18 wei ETH

• Current getEthAmountFromUsd returns:

(200e18 * 1e18) / (3000e18 * 1e18) = 0

• So the user burns AAPL and receives 0 ETH.

• Even after correcting the math, s_ethCollateralPerUser[msg.sender] remains unchanged, so the user’s collateral is still counted after ETH is transferred.

Fix

• Fix getEthAmountFromUsd:

return (usdAmountInWei * PRECISION) / (uint256(price) * ADDITIONAL_FEED_PRECISION);

• Deduct collateral before the health check and transfer:

s_aaplMintedPerUser[msg.sender] -= amountToRedeem;
s_ethCollateralPerUser[msg.sender] -= ethToReturn;

uint256 healthFactor = getHealthFactor(msg.sender);
if (healthFactor < MIN_HEALTH_FACTOR) {
    revert AAPL_feeds__InsufficientCollateral();
}

_burn(msg.sender, amountToRedeem);

(bool success,) = msg.sender.call{value: ethToReturn}("");
if (!success) {
    revert("AAPL_feeds: transfer failed");
}

• Consider adding a reentrancy guard and confirming checks-effects-interactions ordering is preserved