Add read-only reentrancy test

contracts/contracts/mocks/MockEvilReentrantContract.sol

@@ -0,0 +1,141 @@
+// SPDX-License-Identifier: MIT
+pragma solidity ^0.8.0;
+
+import { IVault } from "../interfaces/IVault.sol";
+import { IOracle } from "../interfaces/IOracle.sol";
+import { IRateProvider } from "../interfaces/balancer/IRateProvider.sol";
+
+import { IBalancerVault } from "../interfaces/balancer/IBalancerVault.sol";
+import { IERC20 } from "../utils/InitializableAbstractStrategy.sol";
+
+import { StableMath } from "../utils/StableMath.sol";
+
+import "hardhat/console.sol";
+
+contract MockEvilReentrantContract {
+    using StableMath for uint256;
+
+    IBalancerVault public immutable balancerVault;
+    IERC20 public immutable reth;
+    IERC20 public immutable weth;
+    IVault public immutable oethVault;
+    address public immutable poolAddress;
+    bytes32 public immutable balancerPoolId;
+
+    constructor(
+        address _balancerVault,
+        address _oethVault,
+        address _reth,
+        address _weth,
+        address _poolAddress,
+        bytes32 _poolId
+    ) {
+        balancerVault = IBalancerVault(_balancerVault);
+        oethVault = IVault(_oethVault);
+        reth = IERC20(_reth);
+        weth = IERC20(_weth);
+        poolAddress = _poolAddress;
+        balancerPoolId = _poolId;
+    }
+
+    function doEvilStuff() public {
+        address priceProvider = oethVault.priceProvider();
+        uint256 rethPrice = IOracle(priceProvider).price(address(reth));
+
+        // 1. Join pool
+        uint256[] memory amounts = new uint256[](2);
+        amounts[0] = uint256(10 ether);
+        amounts[1] = rethPrice * 10;
+
+        address[] memory assets = new address[](2);
+        assets[0] = address(reth);
+        assets[1] = address(weth);
+
+        uint256 minBPT = getBPTExpected(assets, amounts).mulTruncate(
+            0.99 ether
+        );
+
+        bytes memory joinUserData = abi.encode(
+            IBalancerVault.WeightedPoolJoinKind.EXACT_TOKENS_IN_FOR_BPT_OUT,
+            amounts,
+            minBPT
+        );
+
+        IBalancerVault.JoinPoolRequest memory joinRequest = IBalancerVault
+            .JoinPoolRequest(assets, amounts, joinUserData, false);
+
+        balancerVault.joinPool(
+            balancerPoolId,
+            address(this),
+            address(this),
+            joinRequest
+        );
+
+        uint256 bptTokenBalance = IERC20(poolAddress).balanceOf(address(this));
+        console.log("BPT Token balance: %s", bptTokenBalance);
+
+        // 2. Redeem as ETH
+        bytes memory exitUserData = abi.encode(
+            IBalancerVault.WeightedPoolExitKind.EXACT_BPT_IN_FOR_ONE_TOKEN_OUT,
+            bptTokenBalance,
+            1
+        );
+
+        assets[1] = address(0); // Receive ETH instead of WETH
+        uint256[] memory exitAmounts = new uint256[](2);
+        exitAmounts[1] = 15 ether;
+        IBalancerVault.ExitPoolRequest memory exitRequest = IBalancerVault
+            .ExitPoolRequest(assets, exitAmounts, exitUserData, false);
+
+        balancerVault.exitPool(
+            balancerPoolId,
+            address(this),
+            payable(address(this)),
+            exitRequest
+        );
+        bptTokenBalance = IERC20(poolAddress).balanceOf(address(this));
+        console.log("BPT Token balance: %s", bptTokenBalance);
+    }
+
+    function getBPTExpected(address[] memory _assets, uint256[] memory _amounts)
+        internal
+        view
+        virtual
+        returns (uint256 bptExpected)
+    {
+        // Get the oracle from the OETH Vault
+        address priceProvider = oethVault.priceProvider();
+
+        for (uint256 i = 0; i < _assets.length; ++i) {
+            uint256 strategyAssetMarketPrice = IOracle(priceProvider).price(
+                _assets[i]
+            );
+            // convert asset amount to ETH amount
+            bptExpected =
+                bptExpected +
+                _amounts[i].mulTruncate(strategyAssetMarketPrice);
+        }
+
+        uint256 bptRate = IRateProvider(poolAddress).getRate();
+        // Convert ETH amount to BPT amount
+        bptExpected = bptExpected.divPrecisely(bptRate);
+    }
+
+    function approveAllTokens() public {
+        // Approve all tokens
+        weth.approve(address(oethVault), type(uint256).max);
+        reth.approve(poolAddress, type(uint256).max);
+        weth.approve(poolAddress, type(uint256).max);
+        reth.approve(address(balancerVault), type(uint256).max);
+        weth.approve(address(balancerVault), type(uint256).max);
+    }
+
+    receive() external payable {
+        console.log("Received ETH");
+
+        // 3. Try to mint OETH
+        oethVault.mint(address(weth), 1 ether, 0.9 ether);
+
+        console.log("You shouldn't see me!!!");
+    }
+}

contracts/contracts/strategies/balancer/BaseAuraStrategy.sol

@@ -5,6 +5,8 @@ pragma solidity ^0.8.0;
  * @title OETH Base Balancer Abstract Strategy
  * @author Origin Protocol Inc
  */
+
+import "@openzeppelin/contracts/utils/math/Math.sol";
 import { BaseBalancerStrategy } from "./BaseBalancerStrategy.sol";
 import { SafeERC20 } from "@openzeppelin/contracts/token/ERC20/utils/SafeERC20.sol";
 import { IRateProvider } from "../../interfaces/balancer/IRateProvider.sol";
@@ -78,8 +80,14 @@ abstract contract BaseAuraStrategy is BaseBalancerStrategy {
      * @param numBPTTokens Number of Balancer Pool Tokens (BPT) to withdraw
      */
     function _lpWithdraw(uint256 numBPTTokens) internal virtual override {
+        // Get all the strategy's BPTs in Aura
+        // maxRedeem is implemented as balanceOf(address) in Aura
+        uint256 maxBPTTokens = IERC4626(auraRewardPoolAddress).maxRedeem(
+            address(this)
+        );
+
         IRewardStaking(auraRewardPoolAddress).withdrawAndUnwrap(
-            numBPTTokens,
+            Math.min(numBPTTokens, maxBPTTokens),
             true // also claim reward tokens
         );
     }
@@ -89,7 +97,9 @@ abstract contract BaseAuraStrategy is BaseBalancerStrategy {
      * the Aura rewards pool to this strategy contract.
      */
     function _lpWithdrawAll() internal virtual override {
-        uint256 bptBalance = IERC4626(auraRewardPoolAddress).balanceOf(
+        // Get all the strategy's BPTs in Aura
+        // maxRedeem is implemented as balanceOf(address) in Aura
+        uint256 bptBalance = IERC4626(auraRewardPoolAddress).maxRedeem(
             address(this)
         );
 
@@ -123,7 +133,8 @@ abstract contract BaseAuraStrategy is BaseBalancerStrategy {
     {
         balancerPoolTokens =
             IERC20(platformAddress).balanceOf(address(this)) +
-            IERC4626(auraRewardPoolAddress).balanceOf(address(this));
+            // maxRedeem is implemented as balanceOf(address) in Aura
+            IERC4626(auraRewardPoolAddress).maxRedeem(address(this));
     }
 
     function _approveBase() internal virtual override {

contracts/contracts/strategies/balancer/BaseBalancerStrategy.sol

@@ -100,12 +100,16 @@ abstract contract BaseBalancerStrategy is InitializableAbstractStrategy {
      * This is not the value (OSUD or ETH) of the assets in the Balancer pool.
      * @param _asset  Address of the Vault collateral asset
      * @return amount  the amount of vault collateral assets
+     * 
+     * IMPORTANT if this function is overridden it needs to have a whenNotInVaultContext
+     * modifier on it or it is susceptible to read-only re-entrancy attack 
      */
     function checkBalance(address _asset)
         external
         view
         virtual
         override
+        whenNotInVaultContext
         returns (uint256 amount)
     {
         // Get the total balance of each of the Balancer pool assets
@@ -137,8 +141,17 @@ abstract contract BaseBalancerStrategy is InitializableAbstractStrategy {
      * Uses the Balancer pool's rate (virtual price) to convert the strategy's
      * Balancer Pool Tokens (BPT) to ETH value.
      * @return value The ETH value
+     * 
+     * IMPORTANT if this function is overridden it needs to have a whenNotInVaultContext
+     * modifier on it or it is susceptible to read-only re-entrancy attack 
      */
-    function checkBalance() external view virtual returns (uint256 value) {
+    function checkBalance()
+        external
+        view
+        virtual
+        whenNotInVaultContext
+        returns (uint256 value)
+    {
         uint256 bptBalance = _getBalancerPoolTokens();
 
         // Convert BPT to ETH value

contracts/test/_fixture.js

@@ -184,6 +184,7 @@ const defaultFixture = deployments.createFixture(async () => {
     LUSDMetaStrategy,
     oethHarvester,
     oethDripper,
+    oethZapper,
     swapper,
     mockSwapper,
     swapper1Inch,
@@ -297,6 +298,8 @@ const defaultFixture = deployments.createFixture(async () => {
       oethDripperProxy.address
     );
 
+    oethZapper = await ethers.getContract("OETHZapper");
+
     // Replace OracelRouter to disable staleness
     const dMockOracleRouterNoStale = await deployWithConfirmation(
       "MockOracleRouterNoStale"
@@ -558,6 +561,7 @@ const defaultFixture = deployments.createFixture(async () => {
     ConvexEthMetaStrategy,
     oethDripper,
     oethHarvester,
+    oethZapper,
     swapper,
     mockSwapper,
     swapper1Inch,
@@ -834,18 +838,20 @@ async function convexVaultFixture() {
 /**
  * Configure a Vault with the balancerREthStrategy
  */
-function balancerREthFixtureSetup() {
+function balancerREthFixtureSetup(config = { defaultStrategy: true }) {
   return deployments.createFixture(async () => {
     const fixture = await defaultFixture();
     const { oethVault, timelock, weth, reth, balancerREthStrategy, josh } =
       fixture;
 
-    await oethVault
-      .connect(timelock)
-      .setAssetDefaultStrategy(reth.address, balancerREthStrategy.address);
-    await oethVault
-      .connect(timelock)
-      .setAssetDefaultStrategy(weth.address, balancerREthStrategy.address);
+    if (config.defaultStrategy) {
+      await oethVault
+        .connect(timelock)
+        .setAssetDefaultStrategy(reth.address, balancerREthStrategy.address);
+      await oethVault
+        .connect(timelock)
+        .setAssetDefaultStrategy(weth.address, balancerREthStrategy.address);
+    }
 
     fixture.rEthBPT = await ethers.getContractAt(
       "IERC20Metadata",
@@ -854,12 +860,30 @@ function balancerREthFixtureSetup() {
     );
     fixture.balancerREthPID = balancer_rETH_WETH_PID;
 
+    fixture.auraPool = await ethers.getContractAt(
+      "IERC4626",
+      addresses.mainnet.rETH_WETH_AuraRewards
+    );
+
     fixture.balancerVault = await ethers.getContractAt(
       "IBalancerVault",
       addresses.mainnet.balancerVault,
       josh
     );
 
+    // Get some rETH from most loaded contracts/wallets
+    await impersonateAndFundAddress(
+      addresses.mainnet.rETH,
+      [
+        "0xCc9EE9483f662091a1de4795249E24aC0aC2630f",
+        "0xC6424e862f1462281B0a5FAc078e4b63006bDEBF",
+        "0x7d6149aD9A573A6E2Ca6eBf7D4897c1B766841B4",
+        "0x7C5aaA2a20b01df027aD032f7A768aC015E77b86",
+        "0x1BeE69b7dFFfA4E2d53C2a2Df135C388AD25dCD2",
+      ],
+      josh.getAddress()
+    );
+
     return fixture;
   });
 }
@@ -1274,7 +1298,9 @@ async function impersonateAndFundContract(address, amount = "100000") {
 
   await _hardhatSetBalance(address, amount);
 
-  return await ethers.provider.getSigner(address);
+  const signer = await ethers.provider.getSigner(address);
+  signer.address = address;
+  return signer;
 }
 
 async function impersonateAndFundAddress(