Fix G-1 G-2 G-3

src/modules/adaptors/Morpho/MorphoAaveV2ATokenAdaptor.sol

@@ -21,14 +21,7 @@ contract MorphoAaveV2ATokenAdaptor is BaseAdaptor, MorphoRewardHandler {
     // Where:
     // `aToken` is the AaveV2 A Token position this adaptor is working with
     //================= Configuration Data Specification =================
-    // isLiquid bool indicating whether user withdraws are allowed from this position.
-    // IMPORTANT: It is possible for a strategist to misconfigure their positions,
-    // and allow user withdraws from aToken positions that are backing loans.
-    // This will be mitigated by only allowing trusted strategists to use this adaptor,
-    // and educating them the dangers of misconfiguring their position.
-    // EX: If a cellar is using their aTokens to back a borrow, then strategists
-    // should make their aToken positions illiquid, so that user withdraws do not
-    // negatively affect the cellars health factor.
+    // NA
     //====================================================================
 
     /**
@@ -116,6 +109,7 @@ contract MorphoAaveV2ATokenAdaptor is BaseAdaptor, MorphoRewardHandler {
 
     /**
      * @notice Uses configuration data to determine if the position is liquid or not.
+     * @param adaptorData the abi encoded aToken address
      */
     function withdrawableFrom(bytes memory adaptorData, bytes memory) public view override returns (uint256) {
         // If position is backing a borrow, then return 0.

src/modules/price-router/Extensions/Balancer/BalancerPoolExtension.sol

@@ -40,24 +40,51 @@ abstract contract BalancerPoolExtension is Extension {
      * here (https://forum.balancer.fi/t/reentrancy-vulnerability-scope-expanded/4345), those functions are unsafe,
      * and subject to manipulation that may result in loss of funds.
      */
-    function _ensureNotInVaultContext(IVault vault) internal view {
-        // Perform the following operation to trigger the Vault's reentrancy guard.
-        // Use a static call so that it can be a view function (even though the
-        // function is non-view).
+    function ensureNotInVaultContext(IVault vault) internal view {
+        // Perform the following operation to trigger the Vault's reentrancy guard:
         //
         // IVault.UserBalanceOp[] memory noop = new IVault.UserBalanceOp[](0);
         // _vault.manageUserBalance(noop);
+        //
+        // However, use a static call so that it can be a view function (even though the function is non-view).
+        // This allows the library to be used more widely, as some functions that need to be protected might be
+        // view.
+        //
+        // This staticcall always reverts, but we need to make sure it doesn't fail due to a re-entrancy attack.
+        // Staticcalls consume all gas forwarded to them on a revert caused by storage modification.
+        // By default, almost the entire available gas is forwarded to the staticcall,
+        // causing the entire call to revert with an 'out of gas' error.
+        //
+        // We set the gas limit to 10k for the staticcall to
+        // avoid wasting gas when it reverts due to storage modification.
+        // `manageUserBalance` is a non-reentrant function in the Vault, so calling it invokes `_enterNonReentrant`
+        // in the `ReentrancyGuard` contract, reproduced here:
+        //
+        //    function _enterNonReentrant() private {
+        //        // If the Vault is actually being reentered, it will revert in the first line, at the `_require` that
+        //        // checks the reentrancy flag, with "BAL#400" (corresponding to Errors.REENTRANCY) in the revertData.
+        //        // The full revertData will be: `abi.encodeWithSignature("Error(string)", "BAL#400")`.
+        //        _require(_status != _ENTERED, Errors.REENTRANCY);
+        //
+        //        // If the Vault is not being reentered, the check above will pass: but it will *still* revert,
+        //        // because the next line attempts to modify storage during a staticcall. However, this type of
+        //        // failure results in empty revertData.
+        //        _status = _ENTERED;
+        //    }
+        //
+        // So based on this analysis, there are only two possible revertData values: empty, or abi.encoded BAL#400.
+        //
+        // It is of course much more bytecode and gas efficient to check for zero-length revertData than to compare it
+        // to the encoded REENTRANCY revertData.
+        //
+        // While it should be impossible for the call to fail in any other way (especially since it reverts before
+        // `manageUserBalance` even gets called), any other error would generate non-zero revertData, so checking for
+        // empty data guards against this case too.
 
-        // solhint-disable-next-line var-name-mixedcase
-        bytes32 REENTRANCY_ERROR_HASH = keccak256(abi.encodeWithSignature("Error(string)", "BAL#400"));
-
-        // read-only re-entrancy protection - this call is always unsuccessful but we need to make sure
-        // it didn't fail due to a re-entrancy attack
-        // This might just look like an issue in foundry. Running a testnet test does not use an insane amount of gas.
-        (, bytes memory revertData) = address(vault).staticcall(
-            abi.encodeWithSelector(vault.manageUserBalance.selector, new address[](0))
+        (, bytes memory revertData) = address(vault).staticcall{ gas: 10_000 }(
+            abi.encodeWithSelector(vault.manageUserBalance.selector, 0)
         );
 
-        if (keccak256(revertData) == REENTRANCY_ERROR_HASH) revert BalancerPoolExtension__Reentrancy();
+        if (revertData.length != 0) revert BalancerPoolExtension__Reentrancy();
     }
 }

src/modules/price-router/Extensions/Balancer/BalancerStablePoolExtension.sol

@@ -101,7 +101,7 @@ contract BalancerStablePoolExtension is BalancerPoolExtension {
      * @param asset the BPT token
      */
     function getPriceInUSD(ERC20 asset) external view override returns (uint256) {
-        _ensureNotInVaultContext(balancerVault);
+        ensureNotInVaultContext(balancerVault);
         IBalancerPool pool = IBalancerPool(address(asset));
 
         // Read extension storage and grab pool tokens

src/modules/price-router/PriceRouter.sol

@@ -471,7 +471,7 @@ contract PriceRouter is Ownable {
 
         uint256 numOfAssets = baseAssets.length;
         exchangeRates = new uint256[](numOfAssets);
-        for (uint256 i; i < numOfAssets; i++) {
+        for (uint256 i; i < numOfAssets; ++i) {
             AssetSettings memory baseSettings = getAssetSettings[baseAssets[i]];
             if (baseSettings.derivative == 0) revert PriceRouter__UnsupportedAsset(address(baseAssets[i]));
             exchangeRates[i] = _getExchangeRate(
@@ -582,7 +582,7 @@ contract PriceRouter is Ownable {
         uint256 valueInQuote;
         uint8 quoteDecimals = quoteAsset.decimals();
 
-        for (uint8 i = 0; i < baseAssets.length; i++) {
+        for (uint256 i = 0; i < baseAssets.length; ++i) {
             // Skip zero amount values.
             if (amounts[i] == 0) continue;
             ERC20 baseAsset = baseAssets[i];