Re-Entrancy Risks Associated With External Calls With Other Liquid Staking Systems.
As part of the strategy to integrate with Liquid Staking tokens for Ethereum staking, the Lybra Protocol vaults are required to make external calls to Liquid Staking systems.\\nFor example, the `depositEtherToMint` function in the vaults makes external calls to deposit Ether and receive the LSD tokens back. While external calls to untrusted third-party contracts may be dangerous, in this case, the Lybra Protocol already extends trust assumptions to these third parties simply through the act of accepting their tokens as collateral. Indeed, in some cases the contract addresses are even hardcoded into the contract and called directly instead of relying on some registry:\\n```\\ncontract LybraWstETHVault is LybraPeUSDVaultBase {\\n Ilido immutable lido;\\n //WstETH = 0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0;\\n //Lido = 0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84;\\n constructor(address \\_lido, address \\_asset, address \\_oracle, address \\_config) LybraPeUSDVaultBase(\\_asset, \\_oracle, \\_config) {\\n lido = Ilido(\\_lido);\\n }\\n\\n function depositEtherToMint(uint256 mintAmount) external payable override {\\n require(msg.value >= 1 ether, "DNL");\\n uint256 sharesAmount = lido.submit{value: msg.value}(address(configurator));\\n require(sharesAmount != 0, "ZERO\\_DEPOSIT");\\n lido.approve(address(collateralAsset), msg.value);\\n uint256 wstETHAmount = IWstETH(address(collateralAsset)).wrap(msg.value);\\n depositedAsset[msg.sender] += wstETHAmount;\\n if (mintAmount > 0) {\\n \\_mintPeUSD(msg.sender, msg.sender, mintAmount, getAssetPrice());\\n }\\n emit DepositEther(msg.sender, address(collateralAsset), msg.value,wstETHAmount, block.timestamp);\\n }\\n```\\n\\nIn that case, depending on the contract, it may be known what contract is being called, and the risk may be assessed as far as what logic may be executed.\\nHowever, in the cases of `BETH` and `rETH`, the calls are being made into a proxy and a contract registry of a DAO (RocketPool's DAO) respectively.\\n```\\ncontract LybraWBETHVault is LybraPeUSDVaultBase {\\n //WBETH = 0xa2e3356610840701bdf5611a53974510ae27e2e1\\n constructor(address \\_asset, address \\_oracle, address \\_config)\\n LybraPeUSDVaultBase(\\_asset, \\_oracle, \\_config) {}\\n\\n function depositEtherToMint(uint256 mintAmount) external payable override {\\n require(msg.value >= 1 ether, "DNL");\\n uint256 preBalance = collateralAsset.balanceOf(address(this));\\n IWBETH(address(collateralAsset)).deposit{value: msg.value}(address(configurator));\\n uint256 balance = collateralAsset.balanceOf(address(this));\\n depositedAsset[msg.sender] += balance - preBalance;\\n\\n if (mintAmount > 0) {\\n \\_mintPeUSD(msg.sender, msg.sender, mintAmount, getAssetPrice());\\n }\\n\\n emit DepositEther(msg.sender, address(collateralAsset), msg.value,balance - preBalance, block.timestamp);\\n }\\n```\\n\\n```\\nconstructor(address \\_rocketStorageAddress, address \\_rETH, address \\_oracle, address \\_config)\\n LybraPeUSDVaultBase(\\_rETH, \\_oracle, \\_config) {\\n rocketStorage = IRocketStorageInterface(\\_rocketStorageAddress);\\n}\\n\\nfunction depositEtherToMint(uint256 mintAmount) external payable override {\\n require(msg.value >= 1 ether, "DNL");\\n uint256 preBalance = collateralAsset.balanceOf(address(this));\\n IRocketDepositPool(rocketStorage.getAddress(keccak256(abi.encodePacked("contract.address", "rocketDepositPool")))).deposit{value: msg.value}();\\n uint256 balance = collateralAsset.balanceOf(address(this));\\n depositedAsset[msg.sender] += balance - preBalance;\\n\\n if (mintAmount > 0) {\\n \\_mintPeUSD(msg.sender, msg.sender, mintAmount, getAssetPrice());\\n }\\n\\n emit DepositEther(msg.sender, address(collateralAsset), msg.value,balance - preBalance, block.timestamp);\\n}\\n```\\n\\nAs a result, it is impossible to make any guarantees for what logic will be executed during the external calls. Namely, reentrancy risks can't be ruled out, and the damage could be critical to the system. While the trust in these parties isn't in question, it would be best practice to avoid any additional reentrancy risks by placing reentrancy guards. Indeed, in the `LybraRETHVault` and `LybraWbETHVault` contracts, one can see the possible damage as the calls are surrounded in a `preBalance <-> balance` pattern.\\nThe whole of third party Liquid Staking systems' operations need not be compromised, only these particular parts would be enough to cause critical damage to the Lybra Protocol.
After conversations with the Lybra Finance team, it has been assessed that reentrancy guards are appropriate in this scenario to avoid any potential reentrancy risk, which is exactly the recommendation this audit team would provide.
```\\ncontract LybraWstETHVault is LybraPeUSDVaultBase {\\n Ilido immutable lido;\\n //WstETH = 0x7f39C581F595B53c5cb19bD0b3f8dA6c935E2Ca0;\\n //Lido = 0xae7ab96520DE3A18E5e111B5EaAb095312D7fE84;\\n constructor(address \\_lido, address \\_asset, address \\_oracle, address \\_config) LybraPeUSDVaultBase(\\_asset, \\_oracle, \\_config) {\\n lido = Ilido(\\_lido);\\n }\\n\\n function depositEtherToMint(uint256 mintAmount) external payable override {\\n require(msg.value >= 1 ether, "DNL");\\n uint256 sharesAmount = lido.submit{value: msg.value}(address(configurator));\\n require(sharesAmount != 0, "ZERO\\_DEPOSIT");\\n lido.approve(address(collateralAsset), msg.value);\\n uint256 wstETHAmount = IWstETH(address(collateralAsset)).wrap(msg.value);\\n depositedAsset[msg.sender] += wstETHAmount;\\n if (mintAmount > 0) {\\n \\_mintPeUSD(msg.sender, msg.sender, mintAmount, getAssetPrice());\\n }\\n emit DepositEther(msg.sender, address(collateralAsset), msg.value,wstETHAmount, block.timestamp);\\n }\\n```\\n