Mismatch between yield amount deposited in shares calculation and `getAccountYieldBalance()`

[c4-bot-1]

Lines of code

https://github.com/code-423n4/2024-07-benddao/blob/main/src/yield/YieldStakingBase.sol#L250-L255 https://github.com/code-423n4/2024-07-benddao/blob/main/src/yield/etherfi/YieldEthStakingEtherfi.sol#L120-L122 https://github.com/code-423n4/2024-07-benddao/blob/main/src/yield/lido/YieldEthStakingLido.sol#L120-L122

Vulnerability details

Impact

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 and Lido. 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. But then, it calculates totalAssets to compute the yield using the actual ETH balance of the positions Etherfi, Lido.

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).

Assessed type

Other

[c4-judge]

MarioPoneder marked the issue as primary issue

[thorseldon]

Fixed at https://github.com/BendDAO/bend-v2/commit/64051b753c3e16b1e7454f258e4c84fbf1effd0e, https://github.com/BendDAO/bend-v2/commit/9525d8eb917981e50f9a96210695d016e42a6e3a.

It's looks like same with 39 & 41.

[MarioPoneder]

Thanks for confirming the duplication, the core issues seemed related but different at the first glance.

[c4-judge]

MarioPoneder marked the issue as satisfactory

[c4-judge]

MarioPoneder marked the issue as selected for report