<!-- canonical: https://0xsimao.com/findings/benddao-unstake-eth-staking-yield -->

# Users cannot unstake from YiedlETHStakingEtherfi.sol, because YieldAccount.sol is incompatible with ether.fi's WithdrawRequestNFT.sol

Crit/High · Code4rena · NFT lending · 19th June, 2024

Finding H-06 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

---

In `YiedlETHStakingEtherfi::stake`, users `yieldBorrow` WETH, convert it to ETH, and stake in Ether.fi's LiquidityPool. Ether.fi's LiquidityPool will mint eETH (protocol token) to the user's `yieldAccount`.

Users will `unstake()` by requesting withdrawal of staked ETH from ether.fi's `WithdrawRequestNFT::requestWithdraw`.

The problem is `WithdrawRequestNFT::requestWithdraw` will `_safeMint` an NFT token to the user's `yieldAccount` as proof of request withdrawal. But the user's `yieldAccount` is not compatible with `_safeMint` because the implementation YieldAcount.sol doesn't have `onERC721Received` method required by `_safeMint`.

```
//src/yield/YieldAccount.sol
...
//@audit YieldAcount doesn't inherit openzeppelin's ERC721Holder.sol, neither does it implement onERC721Received. Not compatible with safeMint method required by ether.fi's WithdrawRequestNFT.
contract YieldAccount is IYieldAccount, Initializable {
...
```

[https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/yield/YieldAccount.sol#L15](https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/yield/YieldAccount.sol#L15)

Flows: [`YieldStakingBase::unstake`](https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/yield/YieldStakingBase.sol#L343) `→`[`YieldEthStakingEtherfi::protocolRequestWithdrawal`](https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/src/yield/etherfi/YieldEthStakingEtherfi.sol#L87) `→` [`liquidityPool::requestWithdraw`](https://github.com/etherfi-protocol/smart-contracts/blob/7c66e571df4fe7ec502a3c325b623bc52349ef9d/src/LiquidityPool.sol#L209) `→` [`WithdrawRequestNFT::requestWithdraw`](https://github.com/etherfi-protocol/smart-contracts/blob/7c66e571df4fe7ec502a3c325b623bc52349ef9d/src/WithdrawRequestNFT.sol#L63) `→` [`_safeMint` (`recipient`, `requestId`)](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/4764ea50750d8bda9096e833706beba86918b163/contracts/token/ERC721/ERC721.sol#L315)

```
    function _safeMint(address to, uint256 tokenId, bytes memory data) internal virtual {
        _mint(to, tokenId);
        require(
            _checkOnERC721Received(address(0), to, tokenId, data),
            "ERC721: transfer to non ERC721Receiver implementer"
        );
    }
```

[https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/ERC721.sol#L315](https://github.com/OpenZeppelin/openzeppelin-contracts/blob/master/contracts/token/ERC721/ERC721.sol#L315)

Because yieldAcount.sol is cloned for every user and atomically used as caller address for ether.fi interactions, no one can unstake from YiedlETHStakingEtherfi.sol. User funds, yield and nft collaterals will be locked. When users' positions become unhealthy (e.g., nft token price drops), `yieldBorrow` debts compounding can also put the protocol at risks.

Note: In the unit test file for `test/yield/YieldEthStakingEtherfi.t.sol`, unit tests passed only because a MockEtherfiWithdrawRequestNFT.sol is used that uses [`_mint()`](https://github.com/code-423n4/2024-07-benddao/blob/117ef61967d4b318fc65170061c9577e674fffa1/test/mocks/MockEtherfiWithdrawRequestNFT.sol#L40) instead of `_safeMint()`.

### Recommended Mitigation Steps

Add onERC721Received support in YieldAccount.sol.

---

Related findings:

- [mTapiocaOFT can't be rebalanced because the Balancer in tapiocaz-audit calls swapETH() or swap() of the RouterETH but does not forward ether for the message fee](https://0xsimao.com/findings/tapioca-dao-rebalanced-balancer-eth-fee): Tapioca DAO
- [Yield form LRTs are forever stuck in the protocol and cannot be withdrawn](https://0xsimao.com/findings/autonomint-yield-form-stuck-withdrawn): Autonomint
- [Wrong rounding direction in StakingPool::unstake() can be abused](https://0xsimao.com/findings/spirit-protocol-rounding-staking-unstake-abused): Spirit Protocol
- [Wrong reward distribution between early and late depositors because of the late `syncRewards()` call in the cycle, `syncReward()` logic should be executed in each withdraw or deposits (without reverting)](https://0xsimao.com/findings/gogopool-reward-rewards-withdraw-deposits): GoGoPool