# Mismatch between yield amount deposited in shares calculation and getAccountYieldBalance() Crit/High · Code4rena · NFT lending · 19th June, 2024 Finding H-01 of the BendDAO security review. - Protocol: https://www.benddao.xyz/ - Report: /reports/benddao - Codebase: https://github.com/0xsimao/2024-07-benddao/tree/117ef61967d4b318fc65170061c9577e674fffa1 - Source: https://code4rena.com/reports/2024-07-benddao --- Mismatch between the return amount in all yield Etherfi and Lido assets and the `getAccountYieldBalance()` call causes the yield amount of each staked nft to the same `YieldAccount` to be incorrect, leading to the possibility of withdrawing the excess for some nfts while being closer to liquidation on others ### Proof of Concept The issue stems from the fact that in `YieldStakingBase::stake()`, on deposit of the borrowed amount to the corresponding yield source (Etherfi or Lido), it returns the minted shares of each integration [Etherfi](https://github.com/etherfi-protocol/smart-contracts/blob/master/src/LiquidityPool.sol#L529C25-L529C48) and [Lido](https://github.com/lidofinance/lido-dao/blob/master/contracts/0.4.24/Lido.sol#L946). As all these integrations shares increase in value over time, on each deposit, less shares than the deposited `ETH` are minted. This will be problematic because `YieldStakingBase` uses the returned shares value from depositing as if they were assets, [here](https://github.com/code-423n4/2024-07-benddao/blob/main/src/yield/YieldStakingBase.sol#L253). But then, it calculates `totalAssets` to compute the yield using the actual ETH balance of the positions [Etherfi](https://github.com/code-423n4/2024-07-benddao/blob/main/src/yield/etherfi/YieldEthStakingEtherfi.sol#L121), [Lido](https://github.com/code-423n4/2024-07-benddao/blob/main/src/yield/lido/YieldEthStakingLido.sol#L121). Due to this, if more than 1 nft is deposited, the borrowed amount and the yield amount will differ significantly. The first staked nft will have a bigger yield amount that can be claimed immediately and the following nfts will have instant losses that may end up being liquidated. Replace the `MockEtherfiLiquidityPool` deposit logic to mimic the real behaviour. It is the same as having an assets/shares ratio in Etherfi of 0.9: ``` function deposit() external payable override returns (uint256) { require(msg.value > 0, 'msg value is 0'); MockeETH(payable(eETH)).mint(msg.sender, msg.value); return msg.value * 9 / 10; } ``` Place the following test in `YieldEthStakingEtherfi.t.sol`. The yield amount of each staked nft is different, but the borrowed amounts are the same. ``` function test_POC_Incorrect_Yield_tracking() public { YieldTestVars memory testVars; prepareWETH(tsDepositor1); uint256[] memory tokenIds = prepareIsolateBAYC(tsBorrower1); uint256 stakeAmount = tsYieldEthStakingEtherfi.getNftValueInUnderlyingAsset(address(tsBAYC)); stakeAmount = (stakeAmount * 80) / 100; tsHEVM.prank(address(tsBorrower1)); address yieldAccount = tsYieldEthStakingEtherfi.createYieldAccount(address(tsBorrower1)); tsHEVM.prank(address(tsBorrower1)); tsYieldEthStakingEtherfi.stake(tsCommonPoolId, address(tsBAYC), tokenIds[0], stakeAmount); tsHEVM.prank(address(tsBorrower1)); tsYieldEthStakingEtherfi.stake(tsCommonPoolId, address(tsBAYC), tokenIds[1], stakeAmount); (testVars.poolId, testVars.state, testVars.debtAmount, testVars.yieldAmount) = tsYieldEthStakingEtherfi .getNftStakeData(address(tsBAYC), tokenIds[0]); assertEq(testVars.yieldAmount, 23433454429562988687); (testVars.poolId, testVars.state, testVars.debtAmount, testVars.yieldAmount) = tsYieldEthStakingEtherfi .getNftStakeData(address(tsBAYC), tokenIds[1]); assertEq(testVars.yieldAmount, 21090108986606689816); } ``` ### Tools Used Vscode, Foundry ### Recommended Mitigation Steps On `YieldStakingBase::deposit()` the claimed amount should be the assets deposited (roughly equal to the borrowed amount minus rounding errors). --- Related findings: - [Total cds deposited amount is incorrectly modified when cds depositor is at a loss, leading to stuck USDa](https://0xsimao.com/findings/autonomint-deposited-modified-depositor-stuck): Autonomint - [`totalCdsDepositedAmountWithOptionFees` is incorrectly reduced in `CDSLib::withdrawUser()`, leading to stuck option fees](https://0xsimao.com/findings/autonomint-fees-reduced-withdraw-stuck): Autonomint - [Liquidation will reduce total cds deposited amount, leading to incorrect option fees](https://0xsimao.com/findings/autonomint-liquidation-reduce-option-fees): Autonomint - [Protected downside is not updated when `cds.getTotalCdsDepositedAmount() < downsideProtected`](https://0xsimao.com/findings/autonomint-protected-downside-updated-deposited): Autonomint