BendDAO Invitational
Findings & Analysis Report

2025-01-22

Overview
- About C4
Summary
Scope
Severity Criteria
High Risk Findings (2)
- [H-01] executeIsolateLiquidate() totalBidAmout/availableLiquidity incorrect accounting
- [H-02] YieldWUSDStaking.repay() may incorrectly over-refund remainAmount to user
Medium Risk Findings (4)
Low Risk and Non-Critical Issues
Mitigation Review
- Mitigation Review scope and summary
- Mitigation errors
Disclosures

Overview

About C4

Code4rena (C4) is an open organization consisting of security researchers, auditors, developers, and individuals with domain expertise in smart contracts.

A C4 audit is an event in which community participants, referred to as Wardens, review, audit, or analyze smart contract logic in exchange for a bounty provided by sponsoring projects.

During the audit outlined in this document, C4 conducted an analysis of the BendDAO smart contract system. The audit took place from December 27, 2024 to January 03, 2025.

This audit was judged by 0xTheC0der.

Following the audit, warden SpicyMeatball reviewed the mitigations implemented by the BendDAO team; the mitigation review report is appended below the audit report.

Final report assembled by Code4rena.

Summary

The C4 analysis yielded an aggregated total of 6 unique vulnerabilities. Of these vulnerabilities, 2 received a risk rating in the category of HIGH severity and 4 received a risk rating in the category of MEDIUM severity.

Additionally, C4 analysis included 2 reports detailing issues with a risk rating of LOW severity or non-critical/informational.

All of the issues presented here are linked back to their original finding.

Scope

The code under review can be found within the C4 BendDAO Invitational repository, and is composed of 21 smart contracts written in the Solidity programming language and includes 5,217 lines of Solidity code.

Severity Criteria

C4 assesses the severity of disclosed vulnerabilities based on three primary risk categories: high, medium, and low/non-critical.

High-level considerations for vulnerabilities span the following key areas when conducting assessments:

Malicious Input Handling
Escalation of privileges
Arithmetic
Gas use

For more information regarding the severity criteria referenced throughout the submission review process, please refer to the documentation provided on the C4 website, specifically our section on Severity Categorization.

High Risk Findings (2)

[H-01] `executeIsolateLiquidate()` `totalBidAmout/availableLiquidity` incorrect accounting

Submitted by bin2chen, also found by oakcobalt (1, 2) and SpicyMeatball (1, 2)

Code reference: IsolateLogic.sol#L499

When executeIsolateLiquidate() is executed, it will account totalBidAmout/availableLiquidity.

The main accounting code is as follows:

    function executeIsolateLiquidate(InputTypes.ExecuteIsolateLiquidateParams memory params) internal {
...

        InterestLogic.updateInterestRates(poolData, debtAssetData, vars.totalBorrowAmount, 0);

        if (vars.totalExtraAmount > 0) {
            // transfer underlying asset from caller to pool
@>          VaultLogic.erc20TransferInLiquidity(debtAssetData, params.msgSender, vars.totalExtraAmount);
        }

        // bid already in pool and now repay the borrow but need to increase liquidity
@>      VaultLogic.erc20TransferOutBidAmountToLiqudity(debtAssetData, vars.totalBorrowAmount);

        // transfer erc721 to winning bidder
        if (params.supplyAsCollateral) {
            VaultLogic.erc721TransferIsolateSupplyOnLiquidate(
                nftAssetData,
                vars.winningBidder,
                params.nftTokenIds,
                true
            );
        } else {
            VaultLogic.erc721DecreaseIsolateSupplyOnLiquidate(nftAssetData, params.nftTokenIds);

            VaultLogic.erc721TransferOutLiquidity(nftAssetData, vars.winningBidder, params.nftTokenIds);
        }

function erc20TransferInLiquidity(DataTypes.AssetData storage assetData, address from, uint256 amount) internal {
        address asset = assetData.underlyingAsset;
        uint256 poolSizeBefore = IERC20Upgradeable(asset).balanceOf(address(this));

@>      assetData.availableLiquidity += amount;

        IERC20Upgradeable(asset).safeTransferFrom(from, address(this), amount);

        uint256 poolSizeAfter = IERC20Upgradeable(asset).balanceOf(address(this));
        require(poolSizeAfter == (poolSizeBefore + amount), Errors.INVALID_TRANSFER_AMOUNT);
    }

    function erc20TransferOutBidAmountToLiqudity(DataTypes.AssetData storage assetData, uint amount) internal {
        require(assetData.totalBidAmout >= amount, Errors.ASSET_INSUFFICIENT_BIDAMOUNT);
@>      assetData.totalBidAmout -= amount;

@>      assetData.availableLiquidity += amount;
    }

We know from the above code that the current formula is as follows:

availableLiquidity += (totalBorrowAmount + totalExtraAmount)
totalBidAmout -= totalBorrowAmount

Both of these accounting errors. TotalExtraAmount is calculated twice.totalBorrowAmount already contains totalExtraAmount.

Example:
Suppose: total Borrow Amount = 100 , Actual Bid Amout = 80
So: total Extra Amount = 20

but in the current algorithm:

availableLiquidity += (totalBorrowAmount + totalExtraAmount) = 100 + 20 = 120
totalBidAmout -= totalBorrowAmount = 100

The correct value is:

availableLiquidity += totalBorrowAmount = 100
totalBidAmout -= (totalBorrowAmount - totalExtraAmount) = (100 - 20) = 80

Impact

availableLiquidity adds extra totalExtraAmount, resulting in inaccurate borrowRate.
totalBidAmout underflow, which makes it impossible to liquidate.

Recommended Mitigation

    function executeIsolateLiquidate(InputTypes.ExecuteIsolateLiquidateParams memory params) internal {
...

        InterestLogic.updateInterestRates(poolData, debtAssetData, vars.totalBorrowAmount, 0);

        if (vars.totalExtraAmount > 0) {
            // transfer underlying asset from caller to pool
            VaultLogic.erc20TransferInLiquidity(debtAssetData, params.msgSender, vars.totalExtraAmount);
        }

        // bid already in pool and now repay the borrow but need to increase liquidity
-       VaultLogic.erc20TransferOutBidAmountToLiqudity(debtAssetData, vars.totalBorrowAmount);
+       VaultLogic.erc20TransferOutBidAmountToLiqudity(debtAssetData, vars.totalBorrowAmount - vars.totalExtraAmount );

thorseldon (BendDAO) confirmed and commented:

Fixed in commit 18a4b84

[H-02] `YieldWUSDStaking.repay()` may incorrectly over-refund `remainAmount` to user

Submitted by bin2chen, also found by oakcobalt and SpicyMeatball

Code reference: YieldWUSDStaking.sol#L468

In YieldWUSDStaking.repay(), we will calculate the amount that needs to be refunded to the user remainAmount:

    function _repay(uint32 poolId, address nft, uint256 tokenId) internal virtual {
...
        // compute fine value
        if (vars.remainAmount >= sd.unstakeFine) {
            vars.remainAmount = vars.remainAmount - sd.unstakeFine;
        } else {
            vars.extraAmount = vars.extraAmount + (sd.unstakeFine - vars.remainAmount);
@>        // missing clear  vars.remainAmount = 0
        }

@>      sd.remainYieldAmount = vars.remainAmount;
...

        // send remain funds to owner
        if (sd.remainYieldAmount > 0) {
@>          underlyingAsset.safeTransfer(vars.nftOwner, sd.remainYieldAmount);
            sd.remainYieldAmount = 0;
        }

The problem is that when use call repay(), if vars.remainAmount < sd.unstakeFine, it doesn’t clear vars.remainAmount to 0, which results in a refund to the user, which should be treated as unstakeFine.

For example: unstakeFine = 10 , remainAmount = 5

As currently calculated

extraAmount = (unstakeFine - remainAmount ) = 10 - 5 = 5
remainYieldAmount = 5 =====> ( should 0)

Impact

Over-refund remainAmount to user.

At the same time, it will also cause the contract balance to be insufficient (after collectFeeToTreasury ()) and others will not be able to repay.

Recommended Mitigation

    function _repay(uint32 poolId, address nft, uint256 tokenId) internal virtual {
...
        // compute fine value
        if (vars.remainAmount >= sd.unstakeFine) {
            vars.remainAmount = vars.remainAmount - sd.unstakeFine;
        } else {
            vars.extraAmount = vars.extraAmount + (sd.unstakeFine - vars.remainAmount);
+           if(msg.sender != botAdmin) {
+               vars.remainAmount = 0;
+           }
        }

        sd.remainYieldAmount = vars.remainAmount;
...

        // send remain funds to owner
        if (sd.remainYieldAmount > 0) {
            underlyingAsset.safeTransfer(vars.nftOwner, sd.remainYieldAmount);
            sd.remainYieldAmount = 0;
        }

thorseldon (BendDAO) confirmed and commented:

Fixed in commit 75055c9

Medium Risk Findings (4)

[M-01] Staked assets can be locked in Lido, due to vulnerable check in `YieldEthStakingLido::protocolDeposit`

Submitted by oakcobalt

Code reference: YieldEthStakingLido.sol#L85

Finding description and impact

In Lido’s WithdrawRequest(WithdrawalQueue.sol) implementation, only a withdraw request with stETH amount within MIN_STETH_WITHDRAWAL_AMOUNT, MAX_STETH_WITHDRAWAL_AMOUNT ( [100, 1000ether]) can be processed.

If the yieldAmount exceeds 1000ether, the withdrawal request has to be split into several requests.

The problem is YiedEthstaingLido::protocolDeposit has a vulnerable check on MAX_STETH_WITHDRAWAL_AMOUNT, and will still allow a position to stake more than MAX_STETH_WITHDRAWAL_AMOUNT overtime.

Since YieldEthStakingLido.sol doesn’t allow splitting withdrawal of a staking position(yieldStakeData), any staking position exceeding MAX_STETH_WITHDRAWAL_AMOUNT cannot be unstaked. Funds will be locked in Lido.

Proof of Concept

Vulnerability: protocolDeposit can be called multiple times through stake(), allowing a user to incrementally stake more asset associated the same staked NFT. The check on the max cap through protocolDeposit is invalid because it only check any single stake amount is below the max cap. Multiple stakes can still exceed the max cap.

//src/yield/lido/YieldEthStakingLido.sol

    function protocolDeposit(YieldStakeData storage sd, uint256 amount) internal virtual override returns (uint256) {
        require(amount >= MIN_STETH_WITHDRAWAL_AMOUNT, Errors.YIELD_ETH_LT_MIN_AMOUNT);
|>      require(amount <= MAX_STETH_WITHDRAWAL_AMOUNT, Errors.YIELD_ETH_GT_MAX_AMOUNT);
        ...

YieldEthStakingLido.sol#L85

//src/yield/YieldStakingBase.sol

    function _stake(uint32 poolId, address nft, uint256 tokenId, uint256 borrowAmount) internal virtual {
...
        // stake in protocol and got the yield
        vars.totalYieldBeforeDeposit = getAccountTotalUnstakedYield(address(vars.yieldAccout));
|>      vars.yieldAmount = protocolDeposit(sd, borrowAmount);
        vars.yieldShare = _convertToYieldSharesWithTotalYield(
            address(vars.yieldAccout),
            vars.yieldAmount,
            vars.totalYieldBeforeDeposit
        );

        // update nft shares
        sd.debtShare += vars.debtShare;
 |>     sd.yieldShare += vars.yieldShare; //@audit-info note: multiple stakes, increment total yieldShare
...

YieldStakingBase.sol#L276

Impact

Unable to unstake, NFT locked and ETH locked in Lido.

Recommended mitigation steps

Consider moving the max cap check (total yield amount + borrow) into YieldStakingBase::_stake. The actual check parameter can be pulled from YieldEthStakingLido.sol. For example,

//src/yield/YieldStakingBase.sol

    function _stake(uint32 poolId, address nft, uint256 tokenId, uint256 borrowAmount) internal virtual {
...
        vars.totalYieldBeforeDeposit = getAccountTotalUnstakedYield(address(vars.yieldAccout));
+       checkMaxStakingCap(vars.totalYieldBeforeDeposit + borrowAmount);
        vars.yieldAmount = protocolDeposit(sd, borrowAmount);
...

//src/yield/lido/YieldEthStakingLido.sol

    function checkMaxStakingCap(uint256 amount ) public pure override{
        require(amount >= MIN_STETH_WITHDRAWAL_AMOUNT, Errors.YIELD_ETH_LT_MIN_AMOUNT);
        require(amount <= MAX_STETH_WITHDRAWAL_AMOUNT, Errors.YIELD_ETH_GT_MAX_AMOUNT);
...

thorseldon (BendDAO) acknowledged and commented:

As we implement the new YieldWUSDStaking contract, we already know the increment stake issues which should not be supported in any yield contract.

But the fix is pending as our frontend only support only 1 stake per NFT for now.

[M-02] `YieldStakingBase::collectFeeToTreasury` will attempt to transfer funds that are not available, resulting in repetitive revert and potential DOS

Submitted by oakcobalt, also found by SpicyMeatball

Code references:

YieldStakingBase::collectFeeToTreasury will attempt to transfer funds that are not available, resulting in repetitive revert.

Proof of Concept

The vulnerability is totalUnstakFine is incremented in the unstake flow by the botAmin. Note that the fund is not available at this point until later when the withdrawal can be claimed in repay flow.

//src/yield/YieldStakingBase.sol

    function _unstake(uint32 poolId, address nft, uint256 tokenId, uint256 unstakeFine) internal virtual {
...
        // sender must be bot or nft owner
        if (msg.sender == botAdmin) {
            require(unstakeFine <= nc.maxUnstakeFine, Errors.YIELD_ETH_EXCEED_MAX_FINE);

            uint256 hf = calculateHealthFactor(nft, nc, sd);
            require(hf < nc.unstakeHeathFactor, Errors.YIELD_ETH_HEATH_FACTOR_TOO_HIGH);

            sd.unstakeFine = unstakeFine;
|>          totalUnstakeFine += unstakeFine;//@audit at this point, unstakeFine is not availalbe because  the position is not claimed until repay() flow.
        }
...

YieldStakingBase.sol#L357

In collectFeeToTreasuy, the transfer amount is totalUnstakeFine - claimedUnstakeFine which includes the funds that are not ready to claim.

//src/yield/YieldStakingBase.sol
    function collectFeeToTreasury() public virtual onlyPoolAdmin {
        address to = addressProvider.getTreasury();
        require(to != address(0), Errors.TREASURY_CANNOT_BE_ZERO);

        if (totalUnstakeFine > claimedUnstakeFine) {
|>          uint256 amountToCollect = totalUnstakeFine - claimedUnstakeFine;
            claimedUnstakeFine += amountToCollect;

            underlyingAsset.safeTransfer(to, amountToCollect);

            emit CollectFeeToTreasury(to, amountToCollect);
        }
    }

YieldStakingBase.sol#L171

Vulnerable cases:

tx racing. Since bot is programmed to call unstake anytime based on the set condition of unhealthy positions, its unstake tx might settle before a pending collectFeeToTreasury by poolAdmin, causing collectFeeToTreasury to revert;

No guarantee that the fines can be repaid. When bot can only partially repay the staking positions, unstakingFines might not be repaid. It’s up to the user to call repay() to fully cover total debts + fines(vars.extraAmount). This causes further uncertainty as to when totalUnstakeFine can be available. And if the user is not able to repay, totalUnstakeFine will never be recovered. This DOSes collectFeeToTreasury.

//src/yield/YieldStakingBase.sol
function _repay(uint32 poolId, address nft, uint256 tokenId) internal virtual {
...
        // compute fine value
    if (vars.remainAmount >= sd.unstakeFine) {
        vars.remainAmount = vars.remainAmount - sd.unstakeFine;
    } else {
|>          vars.extraAmount = vars.extraAmount + (sd.unstakeFine - vars.remainAmount);
    }

    sd.remainYieldAmount = vars.remainAmount;

    // transfer eth from sender exclude bot admin
|>     if ((vars.extraAmount > 0) && (msg.sender != botAdmin)) {
        underlying asset.safeTransferFrom(msg.sender, address(this), vars.extraAmount);
    }

YieldStakingBase.sol#L475-L476

Impact

collectFeeToTreasury might face repetitive revert or even DOS. protocol funds can be locked.

Recommended mitigation steps

Consider only incrementing totalUnstakFine in _repay. The increment amount is the actual funds that are claimed or covered by the user.

thorseldon (BendDAO) acknowledged

[M-03] Missing price staleness check for `WEETH_AGGREGATOR`

Submitted by oakcobalt

Code reference: EETHPriceAdapter.sol#L115

EETHPriceAdapter::latestRoundata() is called by PriceOracle::getAssetPriceFromChainlink. It provides price of eETH in baseCurrency(USD) by using two chainlink oracles - ETH/USD(BASE_AGGREGATOR), WETH/ETH (WEETH_AGGREGATOR).

The problem is the flow of getAssetPriceFromChainlink -> EETHPriceAdapter::latestRoundata() would only have price staleness check for BASE_AGGREGATOR oracle, but no price staleness check for WEETH_AGGREGATOR oracle.

Missing price staleness check for one of the two oracle could result in a invalid eETH/USD price.

Proof of Concept

Flows: PriceOracle::getAssetPriceFromChainlink -> EETHPriceAdapter::latestRoundata

In EETHPriceAdapter::latestRoundData, only the data feed timestamp(updatedAT_) of BASE_AGGREGATOR is passed as return values. WETH_AGGREGATOR is invoked using a simplified method latestAnswer instead of latestRoundData and no meta data of the price feed is passed.

//src/oracles/EETHPriceAdapter.sol
    function latestRoundData()
        public
        view
        returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound)
    {
    ...
            (
            uint80 roundId_,
            int256 basePrice,
            uint256 startedAt_,
            uint256 updatedAt_,
            uint80 answeredInRound_
        ) = BASE_AGGREGATOR.latestRoundData();

|>      int256 eETHBasePrice = _convertWEETHPrice(basePrice, weETHPrice);

        return (roundId_, eETHBasePrice, startedAt_, updatedAt_, answeredInRound_);
    }

EETHPriceAdapter.sol#L115

In PriceOracle::getAssetPriceFromChainlink, only BASE_AGGREGATOR’s price staleness is checked.

//src/PriceOracle.sol

    function getAssetPriceFromChainlink(address asset) public view returns (uint256) {
        AggregatorV2V3Interface sourceAgg = assetChainlinkAggregators[asset];
        require(address(sourceAgg) != address(0), Errors.ASSET_AGGREGATOR_NOT_EXIST);

        (uint80 roundId, int256 answer, , uint256 updatedAt, uint80 answeredInRound) = sourceAgg.latestRoundData();
          //@audit when calling EETHPriceAdapter::latestRoundData, only BASE_AGGREGATOR's staleness will be checked. WEETH_AGGREGATOR's staleness check is missing.
|>        require(answer > 0, Errors.ASSET_PRICE_IS_ZERO);
|>        require(updatedAt != 0, Errors.ORACLE_PRICE_IS_STALE);
|>        require(answeredInRound >= roundId, Errors.ORACLE_PRICE_IS_STALE);

        return uint256(answer);
    }

PriceOracle.sol#L181

Since two chainlink price feeds are used, both price feed’s staleness need to be checked to ensure a valid answer.

Recommended mitigation steps

Consider adding staleness check for WEETH_AGGREGATOR in EETHPriceAdapter::latestRoundata.

thorseldon (BendDAO) acknowledged

0xTheC0der (judge) commented:

Close to M-2 “Use of deprecated chainlink function: latestAnswer()” from the automated findings report, but still different due to staleness consideration. Therefore, valid.

[M-04] `YieldStakingBase.stake()` cannot append `borrowAmount`

Submitted by bin2chen

Code reference: YieldLogic.sol#L150

In this PR, yieldSetERC721TokenData() adds the following restrictions:

    function executeYieldSetERC721TokenData(InputTypes.ExecuteYieldSetERC721TokenDataParams memory params) internal {
...
        if (params.isLock) {
            require(ymData.yieldCap > 0, Errors.YIELD_EXCEED_STAKER_CAP_LIMIT);

@>          require(tokenData.lockerAddr == address(0), Errors.ASSET_ALREADY_LOCKED_IN_USE);

            VaultLogic.erc721SetTokenLockerAddr(nftAssetData, params.tokenId, lockerAddr);
        } else {
            require(tokenData.lockerAddr == lockerAddr, Errors.YIELD_TOKEN_LOCKED_BY_OTHER);

            VaultLogic.erc721SetTokenLockerAddr(nftAssetData, params.tokenId, address(0));
        }

This causes yieldSetERC721TokenData() to be called only once. This is fine for YieldWUSDStaking.sol. But for the other YieldStakingBase.sol, there is a problem, because it is not possible to increase borrowing again (Health Factor is still enough) as before.

In YieldStakingBase.stake()

    function _stake(uint32 poolId, address nft, uint256 tokenId, uint256 borrowAmount) internal virtual {
...
        YieldStakeData storage sd = stakeDatas[nft][tokenId];
        if (sd.yieldAccount == address(0)) {
            require(vars.nftLockerAddr == address(0), Errors.YIELD_ETH_NFT_ALREADY_USED);

            vars.totalDebtAmount = borrowAmount;

            sd.yieldAccount = address(vars.yieldAccout);
            sd.poolId = poolId;
            sd.state = Constants.YIELD_STATUS_ACTIVE;
        } else {
            require(vars.nftLockerAddr == address(this), Errors.YIELD_ETH_NFT_NOT_USED_BY_ME);
            require(sd.state == Constants.YIELD_STATUS_ACTIVE, Errors.YIELD_ETH_STATUS_NOT_ACTIVE);
            require(sd.poolId == poolId, Errors.YIELD_ETH_POOL_NOT_SAME);

            vars.totalDebtAmount = convertToDebtAssets(poolId, sd.debtShare) + borrowAmount;
        }
....
@>      poolYield.yieldSetERC721TokenData(poolId, nft, tokenId, true, address(underlyingAsset));

        // check hf
        uint256 hf = calculateHealthFactor(nft, nc, sd);
        require(hf >= nc.unstakeHeathFactor, Errors.YIELD_ETH_HEATH_FACTOR_TOO_LOW);

        emit Stake(msg.sender, nft, tokenId, borrowAmount);
    }

Impact

Users can’t borrow additional funds and have to pay back the loan first, losing a certain amount of handling fee.

Recommended Mitigation

Two possible modifications

_stake() does not call poolYield.yieldSetERC721TokenData() if appending.

function _stake(uint32 poolId, address nft, uint256 tokenId, uint256 borrowAmount) internal virtual {
...
        require(vars.nftLockerAddr == address(this), Errors.YIELD_ETH_NFT_NOT_USED_BY_ME);
        require(sd.state == Constants.YIELD_STATUS_ACTIVE, Errors.YIELD_ETH_STATUS_NOT_ACTIVE);
        require(sd.poolId == poolId, Errors.YIELD_ETH_POOL_NOT_SAME);

        vars.totalDebtAmount = convertToDebtAssets(poolId, sd.debtShare) + borrowAmount;
    }
....

poolYield.yieldSetERC721TokenData(poolId, nft, tokenId, true, address(underlyingAsset));
if (vars.nftLockerAddr != address(this))
poolYield.yieldSetERC721TokenData(poolId, nft, tokenId, true, address(underlyingAsset));
}

2. `yieldSetERC721TokenData()` to allow the current `nftLockerAddr` to execute.
```diff
    function executeYieldSetERC721TokenData(InputTypes.ExecuteYieldSetERC721TokenDataParams memory params) internal {
...
        if (params.isLock) {
            require(ymData.yieldCap > 0, Errors.YIELD_EXCEED_STAKER_CAP_LIMIT);

-           require(tokenData.lockerAddr == address(0), Errors.ASSET_ALREADY_LOCKED_IN_USE);
+           require(tokenData.lockerAddr == address(0) || tokenData.lockerAddr == lockerAddr), Errors.ASSET_ALREADY_LOCKED_IN_USE);

            VaultLogic.erc721SetTokenLockerAddr(nftAssetData, params.tokenId, lockerAddr);
        } else {
            require(tokenData.lockerAddr == lockerAddr, Errors.YIELD_TOKEN_LOCKED_BY_OTHER);

            VaultLogic.erc721SetTokenLockerAddr(nftAssetData, params.tokenId, address(0));
        }

It is recommended to choose option (2).

thorseldon (BendDAO) confirmed and commented:

Fixed in commit eef87bf

Low Risk and Non-Critical Issues

For this audit, 2 reports were submitted by wardens detailing low risk and non-critical/informational issues. The report highlighted below by oakcobalt received the top score from the judge.

The following warden also submitted a report: bin2chen.

[L-01] Risk of mismatched price used resulting in invalid price

src/oracles/EETHPriceAdapter.sol
Instances(4)

Vulnerabilities

getAnswer will take a roundId input for BASE_AGGREGATOR to get a round-specific price. However, it only takes the latest price for WEETH_AGGREGATOR. Since both prices will be used in convertWEETHPrice, this means when the roundId doesn’t correspond to the latest answer round as defined in WEETH_AGGREGATOR. The resulting eETH price is invalid.

Note: same issue in getRoundata(). There are no checks to ensure the input _roundId matches WEETH_AGGREGATOR.latestAnswer().

Impacts

Invalid price whenever roundId doesn’t correspond to latest round as defined in WETH_AGGREGATOR.

Currently, getAnswer and getRoundata are not embedded in any flow, so no material impact for now. But as a helper function, the implementation will mostly result in an invalid price.

EETHPriceAdapter.sol#L93-L94:

function getAnswer(uint256 roundId) public view returns (int256) {
    int256 basePrice = BASE_AGGREGATOR.getAnswer(roundId);
    int256 weETHPrice = WEETH_AGGREGATOR.latestAnswer();

    return _convertWEETHPrice(basePrice, weETHPrice);
}

EETHPriceAdapter.sol#L136-L138:

function getRoundData(
    uint80 _roundId
)
    public
    view
    returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound)
{
    (
        uint80 roundId_,
        int256 basePrice,
        uint256 startedAt_,
        uint256 updatedAt_,
        uint80 answeredInRound_
    ) = BASE_AGGREGATOR.getRoundData(_roundId);
    int256 weETHPrice = WEETH_AGGREGATOR.latestAnswer();

    int256 eETHBasePrice = _convertWEETHPrice(basePrice, weETHPrice);

Note: similar issue in SUSDSPriceAdapter.sol. No check to ensure roundId’s price is corresponding to the conversion rate from RATE_PROVIDER.

SUSDSPriceAdapter.sol#L76-L77:

function getAnswer(uint256 roundId) public view returns (int256) {
    int256 usdsPrice = USDS_AGGREGATOR.getAnswer(roundId);
    int256 susdsPrice = _convertUSDSPrice(usdsPrice);
    return susdsPrice;
}

SUSDSPriceAdapter.sol#L116-L118:

function getRoundData(
    uint80 _roundId
)
    public
    view
    returns (uint80 roundId, int256 answer, uint256 startedAt, uint256 updatedAt, uint80 answeredInRound)
{
    (
        uint80 roundId_,
        int256 usdsPrice,
        uint256 startedAt_,
        uint256 updatedAt_,
        uint80 answeredInRound_
    ) = USDS_AGGREGATOR.getRoundData(_roundId);

    int256 susdsPrice = _convertUSDSPrice(usdsPrice);

Recommendations

Since each pricefeed has its own roundId increment threshold and intervals, a roundId from two different priceFeeds most likely not the same. Fetching historical data involves searching for roundIds in both aggregator that overlaps in time. If there is a need for historical data search, consider searching based on a timestamp instead of a roundID.

If there is no need for historical data search, consider only provide methods for latest price.

[L-02] Missing price staleness check `inAdapter::latestAnswer`

Instances(2)

There are no staleness checks in multiple latestAnswer methods from the adapter. Due to latestAnswer is not used in flows in scope. No material impact.

[src/oracles/EETHPriceAdapter.sol#L77]((https://github.com/code-423n4/2024-12-benddao/blob/489f8dd0f8e86e5a7550cc6b81f9edfe79efbf4e/src/oracles/EETHPriceAdapter.sol#L77):

function latestAnswer() public view virtual returns (int256) {
    int256 basePrice = BASE_AGGREGATOR.latestAnswer();
    int256 weETHPrice = WEETH_AGGREGATOR.latestAnswer();

    return _convertWEETHPrice(basePrice, weETHPrice);
}

src/oracles/SUSDSPriceAdapter.sol#L62:

function latestAnswer() public view virtual returns (int256) {
    int256 usdsPrice = USDS_AGGREGATOR.latestAnswer();
    return _convertUSDSPrice(usdsPrice);
}

### Recommendation
Consider using latestRoundData and check timestamp is not stale.

## [L-03] Insufficient staleness check for `bendNFTOracle`/`bendTokenOracle`
- src/PriceOracle.sol
- Instances(2)

Simply checking the timestamp !=0, doesn't prevent stale price. PriceOracle's checking that latestTimestamp !=0 is insufficient.

Note that in 07 audit, an insufficient staleness check was raised for a different method `getAssetPriceFromChainlink`. The vulnerable code below in question is added post 07 audit.

1. [PriceOracle.sol#L190](https://github.com/code-423n4/2024-12-benddao/blob/489f8dd0f8e86e5a7550cc6b81f9edfe79efbf4e/src/PriceOracle.sol#L190):
```solidity
    function getAssetPriceFromBendNFTOracle(address asset) public view returns (uint256) {
    
            uint256 updatedAt = bendNFTOracle.getLatestTimestamp(asset);
        require(updatedAt != 0, Errors.ORACLE_PRICE_IS_STALE);

PriceOracle.sol#L209:

function getAssetPriceFromBendTokenOracle(address asset) public view returns (uint256) {

        uint256 updatedAt = bendTokenOracle.getLatestTimestamp(asset);
    require(updatedAt != 0, Errors.ORACLE_PRICE_IS_STALE);

Recommendation

In BendNFTOracle context, it’s recommended for the protocol to establish MAX_NFT_PRICE_UPDATE_INTERVAL . Add adding check (block.timestamp - latestTimestamp) <= MAX_NFT_PRICE_UPDATE_INTERVAL.

[L-04] Incorrect NFT state returned from `getNFTStakeData`

src/yield/YieldStakingBase.sol
Instances(2)

When protocolIsClaimReady, the state might not be claimed yet.Repay needs to be called successfully to change the status to YIELD_STATUS_CLAIM. Risk of incorrect state returned from getNFTStakeData.

State is only changed into CLAIM when a botAdmin or user has repaid.

YieldStakingBase.sol#L547-L548:

function getNftStakeData(
    address nft,
    uint256 tokenId
) public view virtual returns (uint32 poolId, uint8 state, uint256 debtAmount, uint256 yieldAmount) {
...
        if (sd.state == Constants.YIELD_STATUS_UNSTAKE) {
        //@audit When protocolIsClaimReady, the state might not be claim. Risk of incorrect state returned from getNFTStakeData.
        if (protocolIsClaimReady(sd)) {
            state = Constants.YIELD_STATUS_CLAIM;
        }
    }

2. [YieldWUSDStaking.sol#L603-L604](https://github.com/BendDAO/bend-v2/blob/63f0953173acc760323fcbb2414f215a82dd5217/src/yield/wusd/YieldWUSDStaking.sol#L603-L604):
```solidity
    function getNftStakeData(
        address nft,
        uint256 tokenId
    ) public view virtual returns (uint32 poolId, uint8 state, uint256 debtAmount, uint256 yieldAmount) {
        YieldStakeData storage sd = stakeDatas[nft][tokenId];

        state = sd.state;
        if (sd.state == Constants.YIELD_STATUS_UNSTAKE) {
            if (protocolIsClaimReady(sd)) {
                state = Constants.YIELD_STATUS_CLAIM;
            }
        }

Recommendation

Consider removing the if (sd.state == Constants.YIELD_STATUS_UNSTAKE) control flow.

[I-01] Unnecessary code in `VaultLogic.sol`

src/libraries/logic/VaultLogic.sol
Instances (3)

struct tokenData contains three fields - owner, supplyMode, and lockAddr.

When clearing tokenData, setting the three fields to 0 has the same effect as delete assetData.erc721TokenData[tokenIds[i]] which clears the value of struct tokenData.

Currently, there are three instances where both delete and setting fields to 0 are implemented, which are redundant.

VaultLogic.sol#L607:

function erc721DecreaseCrossSupply( … tokenData.owner = address(0); tokenData.supplyMode = 0; tokenData.lockerAddr = address(0); delete assetData.erc721TokenData[tokenIds[i]];

2. [VaultLogic.sol#L640](https://github.com/code-423n4/2024-12-benddao/blob/489f8dd0f8e86e5a7550cc6b81f9edfe79efbf4e/src/libraries/logic/VaultLogic.sol#L640):
```solidity
  function erc721DecreaseIsolateSupply(
  ...
        tokenData.owner = address(0);
      tokenData.supplyMode = 0;
      tokenData.lockerAddr = address(0);
      delete assetData.erc721TokenData[tokenIds[i]];

VaultLogic.sol#L661:

function erc721DecreaseIsolateSupplyOnLiquidate(
...
    tokenData.owner = address(0);
  tokenData.supplyMode = 0;
  tokenData.lockerAddr = address(0);
  delete assetData.erc721TokenData[tokenIds[i]];

Recommendation

Consider removing delete method invocation.

[I-02] Unnecessary code in `YieldAccount.sol`

src/yield/YieldAccount.sol
Instances(1)

There are currently no use cases where YieldAccount will receive ERC1155 or ERC1155 batched. Only onERC721Received is needed for withdrawal flow from ether.fi due to ether.fi mints a withdraw erc721 token as a proof.

//src/yield/YieldAccount.sol

    function onERC1155Received(address, address, uint256, uint256, bytes memory) public virtual returns (bytes4) {
        return this.onERC1155Received.selector;
    }

    function onERC1155BatchReceived(
        address,
        address,
        uint256[] memory,
        uint256[] memory,
        bytes memory
    ) public virtual returns (bytes4) {
        return this.onERC1155BatchReceived.selector;
    }

YieldAccount.sol#L83

Recommendation

Consider removing unnecessary code in yieldAccount.

[I-03] Incorrect comment - pairName is not sDai pair

src/oracles/EETHPriceAdapter.sol
Instances(1)

EETHPriceAdapter provides eETH price and has nothing to do with sDAI. The code comment is incorrect.

    //@audit-info Low: incorrect comment - pairName is the identifier of eETH/ ETH pair not sDai pair.
    /**
     * @param ethAggAddress the address of ETH feed
     * @param weETHAggAddress the address of ETH feed
     * @param weETHAddress the address of the rate provider
|>   * @param pairName the name identifier of sDAI paire
     */
    constructor(address ethAggAddress, address weETHAggAddress, address weETHAddress, string memory pairName) {

EETHPriceAdapter.sol#L52

Recommendation

Change the pairName into eETH.

[I-04] Incorrect comment - this is weETH/ eETH ratio decimal

src/oracles/EETHPriceAdapter.sol
Instances(1)

The code comment for RATIO_DECIMAL is incorrect. The ratio is for weETH/ eETH, not eETH/eETH.

    //@audit-info Low: incorrect comment - this is weETH/ eETH ratio decimal
    /**
|>   * @notice Number of decimals for eETH / eETH ratio
     */
    uint8 public constant RATIO_DECIMALS = 18;

EETHPriceAdapter.sol#L32

Recommendation

Change to number of decimals for WeETH/ eETH ratio

[I-05] Unnecessary typecast - `assetOracleSourceType` returns `uint8` type and can be assigned to `sourceTypes[i]`

src/PriceOracle.sol
Instances(1)

assetOracleSourceType returns uint8 type and can be assigned directly to sourceTypes[i] without typecast

PriceOracle.sol#L90

    function getAssetOracleSourceTypes(address[] calldata assets) public view returns (uint8[] memory sourceTypes) {
        sourceTypes = new uint8[](assets.length);
        for (uint256 i = 0; i < assets.length; i++) {
            //@audit Low?: Unnecessary typecast - assetOracleSourceType returns uint8 type and can be assigned to sourceTypes[i]
            sourceTypes[i] = uint8(assetOracleSourceTypes[assets[i]]);
        }
    }

Recommendations

Change to sourceTypes[i] = assetOracleSourceTypes[assets[i]].

[I-06] `getProtocolTokenPriceInUnderlyingAsset`’s can be simplified to reduce unnecessary computation

src/yield/susds/YieldSavingsUSDS.sol
Instances(1)

getProtocolTokenPriceInUnderlyingAsset uses market price to evaluate the price of sUSDS in USDS token. Since both sUSDS oracle price and USDS oracle price use the same chainlink price fee. The key link here for the sUSDS to USDS price conversion is based on RATE_PROVIDER.chi() which is essentially the sUSDS contract’s share to asset conversion.

This greatly increases the unnecessary complexity and introduces risks of stale chainlink oracles. It should be using sUSDS ‘s USDS -> sUSDS converion (convertToShare) method.

YieldSavingsUSDS.sol#L147-L148:

    function getProtocolTokenPriceInUnderlyingAsset() internal view virtual override returns (uint256) {
        IPriceOracleGetter priceOracle = IPriceOracleGetter(addressProvider.getPriceOracle());
|>      uint256 sUSDSPriceInBase = priceOracle.getAssetPrice(address(susds));
|>      uint256 usdsPriceInBase = priceOracle.getAssetPrice(address(underlyingAsset));
        return sUSDSPriceInBase.mulDiv(10 ** underlyingAsset.decimals(), usdsPriceInBase);
    }

Recommendation

Change the implementation into susds.converToAssets(1 ether).

Mitigation Review

Following the C4 audit, warden SpicyMeatball reviewed the mitigations implemented by the BendDAO team.

Mitigation Review scope and summary

During the mitigation review, SpicyMeatball confirmed that all in-scope findings were mitigated. The table below provides details regarding the status of each in-scope vulnerability from the original audit.

Original finding	Status	Mitigation URL
H-01	🟢 Mitigation Confirmed	commit 18a4b84
H-02	🟢 Mitigation Confirmed	commit 75055c9
M-04	🟢 Mitigation Confirmed	commit eef87bf

Out of scope

All acknowledged findings (M-01, M-02, M-03).

Mitigation errors

One mitigation error was discovered during the mitigation review:

Original finding: H-01 - specifically submission S-3
Link to mitigation

The problematic require statement is still present in IsolateLogic.executeIsolateLiquidate():

    require(
      (vars.totalBorrowAmount + vars.totalRemainAmount) <= vars.totalBidAmount,
      Errors.ISOLATE_LOAN_BORROW_AMOUNT_NOT_COVER
    );

This check is unnecessary, as the subsequent logic ensures that the sender will repay any excess amount, if applicable.

Status: 🟢 The error has been resolved with this commit.

Disclosures

C4 is an open organization governed by participants in the community.

C4 audits incentivize the discovery of exploits, vulnerabilities, and bugs in smart contracts. Security researchers are rewarded at an increasing rate for finding higher-risk issues. Audit submissions are judged by a knowledgeable security researcher and disclosed to sponsoring developers. C4 does not conduct formal verification regarding the provided code but instead provides final verification.

C4 does not provide any guarantee or warranty regarding the security of this project. All smart contract software should be used at the sole risk and responsibility of users.

BendDAO Invitational Findings & Analysis Report

2025-01-22

Table of contents

Overview

About C4

Summary

Scope

Severity Criteria

High Risk Findings (2)

[H-01] executeIsolateLiquidate() totalBidAmout/availableLiquidity incorrect accounting

Impact

Recommended Mitigation

[H-02] YieldWUSDStaking.repay() may incorrectly over-refund remainAmount to user

Impact

Recommended Mitigation

Medium Risk Findings (4)

[M-01] Staked assets can be locked in Lido, due to vulnerable check in YieldEthStakingLido::protocolDeposit

Finding description and impact

Proof of Concept

Impact

Recommended mitigation steps

[M-02] YieldStakingBase::collectFeeToTreasury will attempt to transfer funds that are not available, resulting in repetitive revert and potential DOS

Proof of Concept

Impact

Recommended mitigation steps

[M-03] Missing price staleness check for WEETH_AGGREGATOR

Proof of Concept

Recommended mitigation steps

[M-04] YieldStakingBase.stake() cannot append borrowAmount

Impact

Recommended Mitigation

Low Risk and Non-Critical Issues

[L-01] Risk of mismatched price used resulting in invalid price

Vulnerabilities

Impacts

Recommendations

[L-02] Missing price staleness check inAdapter::latestAnswer

Recommendation

[L-04] Incorrect NFT state returned from getNFTStakeData

Recommendation

[I-01] Unnecessary code in VaultLogic.sol

Recommendation

[I-02] Unnecessary code in YieldAccount.sol

Recommendation

[I-03] Incorrect comment - pairName is not sDai pair

Recommendation

[I-04] Incorrect comment - this is weETH/ eETH ratio decimal

Recommendation

[I-05] Unnecessary typecast - assetOracleSourceType returns uint8 type and can be assigned to sourceTypes[i]

Recommendations

[I-06] getProtocolTokenPriceInUnderlyingAsset’s can be simplified to reduce unnecessary computation

Recommendation

Mitigation Review

Mitigation Review scope and summary

Out of scope

Mitigation errors

Disclosures

BendDAO Invitational
Findings & Analysis Report

[H-01] `executeIsolateLiquidate()` `totalBidAmout/availableLiquidity` incorrect accounting

[H-02] `YieldWUSDStaking.repay()` may incorrectly over-refund `remainAmount` to user

[M-01] Staked assets can be locked in Lido, due to vulnerable check in `YieldEthStakingLido::protocolDeposit`

[M-02] `YieldStakingBase::collectFeeToTreasury` will attempt to transfer funds that are not available, resulting in repetitive revert and potential DOS

[M-03] Missing price staleness check for `WEETH_AGGREGATOR`

[M-04] `YieldStakingBase.stake()` cannot append `borrowAmount`

[L-02] Missing price staleness check `inAdapter::latestAnswer`

[L-04] Incorrect NFT state returned from `getNFTStakeData`

[I-01] Unnecessary code in `VaultLogic.sol`

[I-02] Unnecessary code in `YieldAccount.sol`

[I-05] Unnecessary typecast - `assetOracleSourceType` returns `uint8` type and can be assigned to `sourceTypes[i]`

[I-06] `getProtocolTokenPriceInUnderlyingAsset`’s can be simplified to reduce unnecessary computation