USPD

Core Contracts

6.1 Stabilizer owners can avoid being collateralized by manipulating the global collateralization ratio

For each Stabilizer NFT, a dedicated PositionEscrow contract is deployed to manage the associated collateral. The protocol continuously tracks the global collateralization ratio as a core safety metric, updating it during collateral allocations and removals (e.g., within allocateStabilizerFunds, unallocateStabilizerFunds, and liquidatePosition). For example, in allocateStabilizerFunds:

function allocateStabilizerFunds(
    uint256 ethUsdPrice,
    uint256 priceDecimals
) external payable override returns (AllocationResult memory result) {
    // --SNIP
    // --- Update Snapshot via Reporter ---
    if (result.totalEthEquivalentAdded > 0) {
        reporter.updateSnapshot(int256(result.totalEthEquivalentAdded)); 
    }
    // --SNIP
}

The PositionEscrow contract tracks locked collateral using the ERC20 balanceOf method. Collateral additions are intended to be made through the functions PositionEscrow::addCollateralEth and PositionEscrow::addCollateralStETH, both of which correctly notify the system of new collateral:

function addCollateralStETH(uint256 stETHAmount) external override /* Removed role check */ {
    if (stETHAmount == 0) revert ZeroAmount();

    // Pull stETH from the caller
    bool success = IERC20(stETH).transferFrom(msg.sender, address(this), stETHAmount);
    if (!success) revert TransferFailed(); // Check allowance and balance

    // Emit event acknowledging the stETH added to the pool
    emit CollateralAdded(stETHAmount);

    // ethValue = stEth * yield
    // Report addition to StabilizerNFT
    IStabilizerNFT(stabilizerNFTContract).reportCollateralAddition(stETHAmount);
}

Stabilizer owners can bypass these reporting functions by transferring stETH directly to their PositionEscrow via a standard ERC20 transfer. Since PositionEscrow tracks its collateral balance using the ERC20 balanceOf method, this directly increases the escrow's perceived collateral without triggering any collateral addition report. As a result, the global collateralization ratio becomes underreported.

This underreporting can be exploited in several ways:

1) Consider the following example:

1.a) There are two positions with the following allocations:

• 1st Position: backed cUSPD = $100 | collateral value = $90 | collateralRatio = 90%
• 2nd Position: backed cUSPD = $100 | collateral value = $90 | collateralRatio = 90%

Global system collateralization ratio is 90%

1.b) Both positions can be liquidated, the 1st stabilizer directly transfers stETH to his Position Escrow to increase his collateral value to $110. This position's allocation is updated as follows:

• 1st Position: backed cUSPD = $100 | collateral value = $110 | collateralRatio = 110%
• 2nd Position: backed cUSPD = $100 | collateral value = $90 | collateralRatio = 90%

Crucially, the global system collateralization ratio is still 90% since the last collateral increase was not reported.

1.c) Both positions can still be liquidated. A liquidator submits a TX to liquidate the 1st position. However, given that the system is under-reporting its collateral ratio (90%), the liquidate transaction will revert in the following line, assuming that the position's collateral is healthier than the system, while it is not:

function liquidatePosition(
    uint256 liquidatorTokenId,
    uint256 positionTokenId,
    uint256 cuspdSharesToLiquidate, 
    IPriceOracle.PriceAttestationQuery calldata priceQuery
) external {
    // --SNIP
     if (address(reporter) != address(0)) { 
        uint256 systemCollateralRatio = reporter.getSystemCollateralizationRatio(priceResponse);
        
        if (systemCollateralRatio != type(uint256).max 
            && positionCollateralRatio > systemCollateralRatio) { 
            revert LiquidationNotBelowSystemRatio();
        }
    }
}

Here, the positionCollateralRatio is 110% while the systemCollateralRatio is 90%.

2) Moreover, after directly funding their escrow, a stabilizer can call removeExcessCollateral to withdraw the unreported collateral. The system will report the removal, further desynchronizing the global collateral state, since the addition was never reported.

function removeExcessCollateral(
    address payable recipient,
    uint256 amountToRemove, 
    IPriceOracle.PriceAttestationQuery calldata priceQuery
) external override onlyRole(EXCESSCOLLATERALMANAGER_ROLE) {
    // --SNIP
    // Report removal to StabilizerNFT
    IStabilizerNFT(stabilizerNFTContract).reportCollateralRemoval(amountToRemove);
}

These issues can lead to a denial of service for other users, prevent legitimate liquidations, and severely undermine the protocol’s risk management and collateral accounting.

Recommendation

Consider tracking the stETH locked in EscrowPositions via internal state, this forces stabilizer owners to pass through addCollateralEth and addCollateralStETH where the collateral addition is reported.

6.2 Uniswap ETH/USDC price is incorrectly fetched, causing price deviation checks to always revert

The PriceOracle::attestationService function is used throughout the system to calculate the price of ETH. This function relies on a price attestation from the Morpho oracle and verifies the price against Chainlink and Uniswap as a deviation check:

function attestationService(
    PriceAttestationQuery calldata priceQuery
) public payable whenNotPaused returns (PriceResponse memory) {
    // --SNIP
    if (block.chainid == 1) {
        // Get prices from other sources
        uint256 chainlinkPrice = uint256(
            getChainlinkDataFeedLatestAnswer()
        );
        if (chainlinkPrice == 0) {
            revert PriceSourceUnavailable("Chainlink");
        }

        uint256 uniswapV3Price = getUniswapV3WethUsdcPrice();
        if (uniswapV3Price == 0) {
            revert PriceSourceUnavailable("Uniswap V3");
        }

        // Check price deviations
        if (!_isPriceDeviationAcceptable(priceQuery.price,chainlinkPrice,uniswapV3Price)) {
            revert PriceDeviationTooHigh(priceQuery.price,chainlinkPrice,uniswapV3Price);
        }
    }
    // --SNIP
}

The issue arises in the getUniswapV3WethUsdcPrice function:

function getUniswapV3WethUsdcPrice() public view returns (uint) {
    IUniswapV3Factory factory = IUniswapV3Factory(uniswapV3Factory);
    address uniswapV3PoolWethUSDC = factory.getPool(
        uniswapRouter.WETH(),
        usdcAddress,
        3000
    );
    if (uniswapV3PoolWethUSDC != address(0)) {
        IUniswapV3PoolState uniswapPoolState = IUniswapV3PoolState(uniswapV3PoolWethUSDC);
        (uint sqrtPriceX96, , , , , , ) = uniswapPoolState.slot0();
==>        return ((sqrtPriceX96 / 2 ** 96) ** 2) * 1e12;
    }

    return 0;
}

The function incorrectly assumes that sqrtPriceX96 from slot0 gives the price of ETH in terms of USDC, when in reality, for USDC/WETH pool, it gives the price of USDC in terms of ETH (because, in the USDC/WETH pool, USDC is token0). As a result, the returned price is inverted and significantly larger than the actual ETH price in USDC. This leads to the price deviation check always reverting, effectively causing a permanent denial of service to the protocol.

Recommendation

Consider correcting the implementation to invert the price so it accurately reflects the price of ETH in terms of USDC.

6.3 MINIMUM_UNALLOCATE_COLLATERALIZATION_RATIO is incorrectly set, rendering the system collateralization ratio check ineffective

When burning cUSPD, the unallocateStabilizerFunds function is called to withdraw the corresponding collateral. Before proceeding, the function checks that the global system collateralization ratio is healthy (i.e., above 100%):

function unallocateStabilizerFunds(
    uint256 poolSharesToUnallocate, // Changed parameter name
    IPriceOracle.PriceResponse memory priceResponse
) external override returns (uint256 unallocatedEth) {
    // --SNIP
    if (address(reporter) != address(0)) {
        uint256 currentSystemRatio = reporter.getSystemCollateralizationRatio(priceResponse);
        if (currentSystemRatio < MINIMUM_UNALLOCATE_COLLATERALIZATION_RATIO) {
            revert SystemUnstableUnallocationNotAllowed();
        }
    }
}

However, there is a critical misalignment between the scaling of the value returned by reporter.getSystemCollateralizationRatio and the value of MINIMUM_UNALLOCATE_COLLATERALIZATION_RATIO. The former is scaled by 10000, while the latter is not:

uint256 public constant MINIMUM_UNALLOCATE_COLLATERALIZATION_RATIO = 100; // e.g., 100 for 100%

As a result, the current value of 100 actually represents a 1% collateralization ratio when compared against the scaled currentSystemRatio, effectively making the collateralization check meaningless.

Recommendation

Consider updating the value of MINIMUM_UNALLOCATE_COLLATERALIZATION_RATIO with the scaling used in the return value of getSystemCollateralizationRatio.

6.4 Transferring the Stabilizer NFT does not transfer the EXCESSCOLLATERALMANAGER_ROLE in the PositionEscrow contract

When a user mints a stabilizer NFT, two contracts are initialized: PositionEscrow and StabilizerEscrow. During initialization, the EXCESSCOLLATERALMANAGER_ROLE—which permits the holder to withdraw excess collateral via removeExcessCollateral—is granted to the stabilizer NFT owner:

function initialize( // <-- Renamed from constructor
    address _stabilizerNFT,
    address _stabilizerOwner,
    address _stETHAddress,
    address _lidoAddress,
    address _rateContractAddress,
    address _oracleAddress
) external initializer { // <-- Added initializer modifier
    __AccessControl_init(); // <-- Initialize AccessControl

    if (_stabilizerNFT == address(0) || _stabilizerOwner == address(0) || _stETHAddress == address(0) || _lidoAddress == address(0) || _rateContractAddress == address(0) || _oracleAddress == address(0)) {
        revert ZeroAddress();
    }

    stabilizerNFTContract = _stabilizerNFT;
    stETH = _stETHAddress;
    lido = _lidoAddress; 
    rateContract = _rateContractAddress;
    oracle = _oracleAddress;

    _grantRole(DEFAULT_ADMIN_ROLE, _stabilizerNFT);
    _grantRole(STABILIZER_ROLE, _stabilizerNFT);
    _grantRole(EXCESSCOLLATERALMANAGER_ROLE, _stabilizerOwner);
}

However, when the stabilizer NFT is transferred to a new account, the EXCESSCOLLATERALMANAGER_ROLE is not revoked from the previous owner nor granted to the new owner. The _update function, which is invoked on transfers, does not handle role updates:

// StabilizerNFT.sol

function _update(address to, uint256 tokenId, address auth)
    internal
    override(ERC721Upgradeable, ERC721EnumerableUpgradeable)
    returns (address)
{
    return super._update(to, tokenId, auth);
}

This allows the previous owner to continue withdrawing excess collateral, while the new owner does not gain the necessary permissions.

Recommendation

Consider revoking the EXCESSCOLLATERALMANAGER_ROLE role from the old owner and grant it to the new one upon NFT transfer.

6.5 Unallocated shares from unallocateStabilizerFunds are not refunded to the user and are permanently lost

When users burn their cUSPD tokens by calling cUSPDToken::burnShares, the function immediately burns the specified number of shares and then calls StabilizerNFT::unallocateStabilizerFunds to unallocate the corresponding collateral and refund it to the user:

function burnShares(
    uint256 sharesAmount,
    address payable to,
    IPriceOracle.PriceAttestationQuery calldata priceQuery
) external returns (uint256 unallocatedStEthReturned) {
    // --SNIP
==>    _burn(msg.sender, sharesAmount);

    // 3. Unallocate funds via StabilizerNFT
    uint256 unallocatedStEth = stabilizer.unallocateStabilizerFunds(
        sharesAmount,
        oracleResponse
    );
    // --SNIP

}

Within unallocateStabilizerFunds, the function iterates backward over the allocated positions to withdraw the equivalent ETH of the burned shares. Notably, the implementation enforces a minimum remaining gas requirement before continuing the loop, which is intended to prevent DoS condition in cases with a large number of allocated positions:

function unallocateStabilizerFunds(
    uint256 poolSharesToUnallocate, // Changed parameter name
    IPriceOracle.PriceResponse memory priceResponse
) external override returns (uint256 unallocatedEth) {
    // --SNIP
    uint256 currentId = highestAllocatedId;
    uint256 remainingPoolShares = poolSharesToUnallocate;
    uint256 totalUserStEthReturned; //default = 0;
    uint256 totalEthEquivalentRemovedAggregate; //default = 0;

    while (currentId != 0 && remainingPoolShares > 0) {
        if (gasleft() < MIN_GAS) break;
        // --SNIP
    }
     require(totalUserStEthReturned > 0, "No funds unallocated");

    if (totalEthEquivalentRemovedAggregate > 0) {
        reporter.updateSnapshot(-int256(totalEthEquivalentRemovedAggregate)); 
    }

    return totalUserStEthReturned;
}

If the loop exits early due to gas exhaustion, the function may not fully unallocate all of the shares passed in poolSharesToUnallocate. However, since the entire amount is already burned in cUSPDToken::burnShares before calling unallocateStabilizerFunds, any unprocessed shares are neither refunded to the user nor retained, resulting in a permanent loss of those shares.

Recommendation

Consider modifying the logic to burn only the shares that have been successfully unallocated in the StabilizerNFT

6.6 Malicious Stabilizers can permanently DoS the protocol

When minting cUSPD, the token contract calls StabilizerNFT::allocateStabilizerFunds to over-collateralize the minted tokens. The function iterates over unallocated positions to back the new tokens:

function allocateStabilizerFunds(
    // poolSharesToMint removed
    uint256 ethUsdPrice,
    uint256 priceDecimals
) external payable override returns (AllocationResult memory result) {
    require(msg.sender == address(cuspdToken), "Only cUSPD contract"); // Check against cUSPD
    require(lowestUnallocatedId != 0, "No unallocated funds");
    require(msg.value > 0, "No ETH sent"); // User must send ETH

    uint256 currentId = lowestUnallocatedId;
    uint256 remainingEth = msg.value;
    while (currentId != 0 && remainingEth > 0) {
        // --SNIP
    }
}

For each unallocated position, the function determines the required collateral, withdraws it from the StabilizerEscrow, and attempts to deposit it into the PositionEscrow:

uint256 stabilizerStEthNeeded = (remainingEth * (pos.minCollateralRatio - 10000)) / 10000;
// Determine how much stabilizer stETH can actually be allocated (min of needed and available)
uint256 toAllocate = stabilizerStEthNeeded > escrowBalance
    ? escrowBalance
    : stabilizerStEthNeeded;

// If stabilizer can't provide the ideally needed amount, adjust the user's ETH share accordingly
uint256 userEthShare = remainingEth;
if (toAllocate < stabilizerStEthNeeded) {
    userEthShare = (toAllocate * 10000) / (pos.minCollateralRatio - 10000);

    if (userEthShare > remainingEth) {
        userEthShare = remainingEth;
    }
}

// --- Interact with PositionEscrow ---
// PositionNFT interaction removed
address positionEscrowAddress = positionEscrows[currentId];

// 1. Transfer Stabilizer's stETH from StabilizerEscrow to PositionEscrow
// Approve this contract to pull from StabilizerEscrow
IStabilizerEscrow(escrowAddress).approveAllocation(toAllocate, address(this));
// Pull the funds
bool successStabilizer = IERC20(stETH).transferFrom(escrowAddress, positionEscrowAddress, toAllocate);
if (!successStabilizer) revert("Stabilizer stETH transfer to PositionEscrow failed");

// 2. Call PositionEscrow.addCollateralFromStabilizer
// This sends the user's ETH (userEthShare) which gets converted to stETH inside PositionEscrow,
// and acknowledges the stabilizer's stETH (toAllocate) that we just transferred.
IPositionEscrow(positionEscrowAddress).addCollateralFromStabilizer{value: userEthShare}(toAllocate);

The addCollateralFromStabilizer function in PositionEscrow stakes the ETH into Lido and reverts if either staking fails or the returned stETH is zero:

function addCollateralFromStabilizer(uint256 stabilizerStEthAmount)
    external
    payable
    override
    onlyRole(STABILIZER_ROLE)
{
    uint256 userEthAmount = msg.value;
    if (userEthAmount == 0 && stabilizerStEthAmount == 0) revert ZeroAmount(); // Must add something

    uint256 userStEthReceived; //default 0
    if (userEthAmount > 0) {
        // Stake User's ETH via Lido - stETH is minted directly to this contract
        try ILido(lido).submit{value: userEthAmount}(address(0)) returns (uint256 receivedStEth) {
            userStEthReceived = receivedStEth;
==>            if (userStEthReceived == 0) revert TransferFailed(); // Lido submit should return > 0 stETH
        } catch {
            revert TransferFailed(); // Lido submit failed
        }
    }

    // --SNIP
}

Since userEthShare is ultimately determined by the available collateral in a stabilizer’s position ( min(unallocated funds, msg.value):

// StabilizerNFT::allocateStabilizerFunds

uint256 toAllocate = stabilizerStEthNeeded > escrowBalance
    ? escrowBalance
    : stabilizerStEthNeeded;

a malicious stabilizer can create a position with an extremely small amount of collateral (e.g., 1 wei). During allocation, this minimal value will be submitted to Lido for staking. Due to the internal Lido logic, staking such a small amount will return zero stETH, causing addCollateralFromStabilizer to revert and thus reverting the entire mint transaction:

// Lido.sol

function getSharesByPooledEth(uint256 _ethAmount) public view returns (uint256) {
    return _ethAmount
        .mul(_getTotalShares())
        .div(_getTotalPooledEther());
}

At the time of writing, the value of _getTotalShares and _getTotalPooledEther are 7536473279201896554811148 and 9107424450461297259544963, respectively. So, staking 1 wei would result in zero stETH due to Lido’s proportional share calculation.

Because the allocation logic always processes positions starting from the lowest unallocated position and there is no privileged mechanism to remove or skip malicious positions, a single malicious position can indefinitely block the minting of new cUSPD tokens, permanently DoSing to the protocol.

Recommendation

Consider enforcing a minimum collateral amount for stabilizer positions

6.7 Stabilizers Can Set a Minimum Collateralization Ratio That Makes Their Positions Immediately Liquidatable

The StabilizerNFT contract allows a stabilizer to set a minimum collateralization ratio ( minCollateralRatio) for their position using the setMinCollateralizationRatio(...) function. This ratio determines the collateral level when their funds are allocated via allocateStabilizerFunds(...). Another feature, liquidatePosition(...), allows users to liquidate positions that are under a specific collateralization threshold.

The problem is a logical flaw where the minimum allowed minCollateralRatio is lower than the liquidation thresholds available to lower NFT Id holders. A stabilizer can set their minCollateralRatio to 11000 (110%), which is the lowest permitted value.

function setMinCollateralizationRatio(
    uint256 tokenId,
    uint256 newRatio
) external {
    require(ownerOf(tokenId) == msg.sender, "Not token owner");
    // @audit-issue A user can set a ratio of 11000 (110%).
    require(newRatio >= 11000 && newRatio <= 100000, "Ratio must be at between 110.00% and 1000%");
    // ...
    positions[tokenId].minCollateralRatio = newRatio;
    // ...
}

However, the liquidation system gives an advantage to holders of low-numbered NFTs. The owner of liquidatorTokenId 1 can liquidate any position with a collateral ratio below 12500 (125%), and this threshold decreases for higher token IDs.

function liquidatePosition(...) external {
    // ...
    uint256 calculatedThreshold;
    if (liquidatorTokenId == 0) {
        calculatedThreshold = 11000;
    } else {
        // ...
        if (((liquidatorTokenId - 1) * 50) >= (12500 - 11000)) { 
             calculatedThreshold = 11000;
        } else {
            // @audit The threshold can be as high as 12500 for liquidatorTokenId 1.
            calculatedThreshold = 12500 - ((liquidatorTokenId - 1) * 50);
        }
    }

    // @audit A position with a ratio of 11000 will pass this check if the
    // liquidator has a privileged token ID (e.g., ID 1-30).
    require(
        positionCollateralRatio < calculatedThreshold,
        "Position not below liquidation threshold"
    );
    // ...
}

This creates a scenario where a stabilizer setting their minimum collateralization ratio to 110% will find their position immediately liquidatable by any NFT holder with an ID between 1 and 30, because their set minimum (110%) is below the liquidation thresholds for those NFT IDs (e.g., ID 30's threshold is 110.5%). These positions become available for immediate liquidation, despite the stabilizer's intention to set a reasonable minimum.

Recommendation

Consider increasing the minimum value for newRatio in the setMinCollateralizationRatio(...) function. The minimum should be greater than the highest possible liquidation threshold.

6.8 Misconfigured Signer Can Cause Denial of Service in attestationService(...)

The attestationService(...) function in the PriceOracle contract is called by other protocol contracts to verify signed price data supplied by end-users. The function uses the priceQuery.dataTimestamp field to prevent two types of issues: data being too old (staleness) and old data being replayed after newer data has already been submitted.

The function correctly checks that the provided dataTimestamp is not too far in the past relative to block.timestamp. However, it lacks a corresponding check to ensure the dataTimestamp is not unreasonably far in the future.

function attestationService(
    PriceAttestationQuery calldata priceQuery
) public payable whenNotPaused returns (PriceResponse memory) {
    // ...

    // @audit A misconfigured signer can set `lastAttestationTimestamp` to a large future value,
    // causing this check to fail for all subsequent legitimate calls.
    if ((priceQuery.dataTimestamp + 1000 * 15) < lastAttestationTimestamp) {
        revert StaleAttestation(
            lastAttestationTimestamp,
            priceQuery.dataTimestamp
        );
    }
    
    // ...

    // @audit This update allows a misconfigured signer to poison the contract's state
    // with a large future timestamp, breaking the replay protection.
    lastAttestationTimestamp = priceQuery.dataTimestamp;
    // ...
}

This could lead to a DoS if a signer is misconfigured. For instance, a bug in the off-chain signing software or an incorrect system clock could cause a signer to submit an attestation with a large future timestamp. If this transaction is processed, it would set lastAttestationTimestamp to this incorrect future value. Consequently, all subsequent attestations from correctly configured signers would fail the replay protection check causing them to revert. This would unintentionally halt all price updates and disrupt protocol operations that rely on the oracle

Recommendation

Consider adding an upper-bound check on priceQuery.dataTimestamp within the attestationService(...) function.

6.9 Stale yield factor from queued messages can devalue L2 USPD

The protocol allows USPD tokens to be bridged from L1 to L2 through Wormhole's NTTManager (Native Token Transfer Manager) contract. When tokens are bridged, the message payload includes the token amount and the current L1 yieldFactor. On L2, the releaseShares(...) function in the BridgeEscrow contract mints the bridged tokens and updates the L2's yield factor to match the one sent from L1.

function releaseShares(...) external nonReentrant {
// ...
    if (block.chainid == MAINNET_CHAIN_ID) {
        // ...
    } else {
        // L2 (Satellite Chain): Bridging from L1 to L2.
        IcUSPDToken(address(cUSPDToken)).mint(recipient, cUSPDShareAmount);
        totalBridgedInShares += cUSPDShareAmount;

        // @audit The yield factor from the message is used to update the rate contract.
        rateContract.updateL2YieldFactor(l2YieldFactor);
    }
// ...
}

The Wormhole NTTManager contract has a rate-limiting feature. If the rate of incoming transfers to an L2 chain goes over a set limit, the transfer message is not handled right away but is placed in a queue. This queued message contains the yieldFactor from when it was started on L1 and can be run later by anyone.

// NttManager.sol

function _handleTransfer(
    uint16 sourceChainId,
    bytes32 sourceNttManagerAddress,
    TransceiverStructs.NttManagerMessage memory message,
    bytes32 digest
) internal {
    TransceiverStructs.NativeTokenTransfer memory nativeTokenTransfer =
        TransceiverStructs.parseNativeTokenTransfer(message.payload);

    // verify that the destination chain is valid
    if (nativeTokenTransfer.toChain != chainId) {
        revert InvalidTargetChain(nativeTokenTransfer.toChain, chainId);
    }
    uint8 toDecimals = tokenDecimals();
    TrimmedAmount nativeTransferAmount = (nativeTokenTransfer.amount.untrim(toDecimals)).trim(toDecimals, toDecimals);

    address transferRecipient = fromWormholeFormat(nativeTokenTransfer.to);

    uint256 receivedYield = _handleAdditionalPayload(
        sourceChainId, sourceNttManagerAddress, message.id, message.sender, nativeTokenTransfer
    );

    bool enqueued = _enqueueOrConsumeInboundRateLimit(
        digest, sourceChainId, nativeTransferAmount, transferRecipient, receivedYield
    );

==>       if (enqueued) {
        return;
    }

    _mintOrUnlockToRecipient(
        digest, transferRecipient, nativeTransferAmount, false, sourceChainId, receivedYield
    );
}

This creates a situation where a delayed bridge message can update the L2 yieldFactor with a stale value. For example, a transfer is started on L1, but due to rate limits, its execution is queued on L2. While it's in the queue, other transfers are handled, and the L2 yieldFactor is correctly updated. If the queued message with the old (and potentially lower) yieldFactor is executed, it'll overwrite the correct/the most up-to-date yieldFactor value.

This would cause the USPDToken.balanceOf(...) function on L2 to return lower values for all holders, as the balance depends on shares multiplied by the yield factor. If USPD token on L2 is used as collateral in DeFi protocols for lending and borrowing, this sudden change in the USPD balance can lead to liquidation of user positions.

Recommendation

Consider implementing staleness checks for yieldFactor on L2. This staleness check would prevent an older message from updating the more recent yieldFactor.

Resolution

We had a staleness check in there that prevented a lower yieldFactor being set than the existing one. Now, we removed that, because in an unknown situation where the stETH yield actually gets negative for some reason, we are still stuck with the lockstep for stETH yield and if the stETH yield is negative so is ours. It is going to be a known issue, no fix for the time being.

6.10 The function setLiquidationParameters should enforce _payoutPercent to be strictly greater than 100

The setLiquidationParameters function allows the admin to update the value of liquidationLiquidatorPayoutPercent, which determines the reward payout for liquidators when they liquidate under-collateralized positions:

function setLiquidationParameters(uint256 _payoutPercent /* Removed _thresholdPercent */) external onlyRole(DEFAULT_ADMIN_ROLE) {
==>    require(_payoutPercent >= 100, "Payout percent must be >= 100");

    liquidationLiquidatorPayoutPercent = _payoutPercent;
    emit LiquidationParametersUpdated(_payoutPercent);
}

Currently, the function permits setting the payout percent to exactly 100%, which would mean liquidators receive no incentive for performing liquidations. This will discourage participation from liquidators.

Recommendation

Consider updating the validation check to require _payoutPercent to be strictly greater than 100, ensuring that liquidators always receive a positive incentive for their participation.

6.11 Address is used before being validated

In the StabilizerNFT contract's liquidatePosition(...) function, it fetches the current price of stETH from an oracle contract. The address of this oracle is retrieved by calling cuspdToken.oracle().

The function first attempts to call the attestationService(...) function on the oracle address returned by cuspdToken.oracle(). Only after this call does it check whether the returned oracle address is a non-zero address. The addresses should be validated before they are used in interactions.

function liquidatePosition(
    // ...
) external {
    // ...
    // @audit The address returned by `cuspdToken.oracle()` is used before it is validated.
    IPriceOracle.PriceResponse memory priceResponse = IPriceOracle(cuspdToken.oracle()).attestationService(priceQuery);
    // @audit The check for a zero address occurs after the address has already been used.
    require(address(cuspdToken.oracle()) != address(0), "Oracle not set in cUSPDToken"); 
    require(priceResponse.price > 0, "Invalid oracle price");
    // ...
}

Recommendation

Consider validating the oracle address before using it.

6.12 The shortfall condition in liquidatePosition is always true and can safely be removed

In the liquidatePosition(...) function, the logic for distributing collateral to a liquidator is split into two main branches. The else block at line is executed only when the actual backing collateral is insufficient to cover the liquidator's target payout ( actualBackingStEth < targetPayoutToLiquidator).

However, inside this else block, another check is performed:

function liquidatePosition(
    // ...
) external {
    // ...
    {
        // ...
        if (actualBackingStEth >= targetPayoutToLiquidator) {
            // ...
        } else {
            // ...

            // @audit This check is always true because the code is in the `else`
            // block of the check above.
==>            if (targetPayoutToLiquidator > actualBackingStEth) { 
                uint256 shortfallAmount = targetPayoutToLiquidator - actualBackingStEth;
                // ...
            }
        }
    } 
    // ...
}

This condition is guaranteed to be true because it is the same condition that caused this branch of the code to be executed in the first place.

This redundant check consumes unnecessary gas and adds a small amount of bytecode.

Recommendation

Consider removing the redundant if condition.