[SOV-4416] apply reentrancy guard zero

contracts/BorrowerOperations.sol

@@ -1,5 +1,4 @@
 // SPDX-License-Identifier: MIT
-
 pragma solidity 0.6.11;
 pragma experimental ABIEncoderV2;
 
@@ -159,6 +158,25 @@ contract BorrowerOperations is
         emit ZEROStakingAddressChanged(_zeroStakingAddress);
     }
 
+    /*
+     * @notice set the user's block number for opening or increasing LoCs, and do check & reset to 0 for closing or decreasing
+     *
+     * @dev This is the function will be called by the open, close, increase, decrease trove function
+     */
+    function recoveryModeMutexHandler(bool _openOrIncrease) private {
+        if(_openOrIncrease) {
+            recoveryModeMutex[msg.sender] = block.number;
+        } else {
+            if(recoveryModeMutex[msg.sender] > 0) {
+                if(recoveryModeMutex[msg.sender] == block.number) {
+                    revert("Recovery mode mutex locked. Try in another block");
+                }
+
+                recoveryModeMutex[msg.sender] = 0;
+            }
+        }
+    }
+
     function setMassetManagerAddress(address _massetManagerAddress) external onlyOwner {
         massetManager = IMassetManager(_massetManagerAddress);
         emit MassetManagerAddressChanged(_massetManagerAddress);
@@ -203,6 +221,10 @@ contract BorrowerOperations is
         vars.price = priceFeed.fetchPrice();
         bool isRecoveryMode = _checkRecoveryMode(vars.price);
 
+        if(isRecoveryMode && _ZUSDAmount > 0) {
+            recoveryModeMutexHandler(true);
+        }
+
         _requireValidMaxFeePercentage(_maxFeePercentage, isRecoveryMode);
         _requireTroveisNotActive(contractsCache.troveManager, msg.sender);
 
@@ -585,6 +607,10 @@ contract BorrowerOperations is
         vars.price = priceFeed.fetchPrice();
         vars.isRecoveryMode = _checkRecoveryMode(vars.price);
 
+        if(vars.isRecoveryMode && _ZUSDChange > 0) {
+            recoveryModeMutexHandler(_isDebtIncrease);
+        }
+
         if (_isDebtIncrease) {
             _requireValidMaxFeePercentage(_maxFeePercentage, vars.isRecoveryMode);
             _requireNonZeroDebtChange(_ZUSDChange);

contracts/BorrowerOperationsStorage.sol

@@ -36,4 +36,9 @@ contract BorrowerOperationsStorage is Ownable {
 
     IMassetManager public massetManager;
     IFeeDistributor public feeDistributor;
+
+    /*
+     * Store the LoC block number on open/increase when in the Recovery mode
+     */
+    mapping(address => uint256) public recoveryModeMutex;
 }

contracts/TestContracts/BorrowerOperationsCrossReentrancy.sol

@@ -0,0 +1,89 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.6.11;
+
+import "../Interfaces/IBorrowerOperations.sol";
+import "hardhat/console.sol";
+
+interface IPriceFeedTestnet {
+    function setPrice(uint256 price) external returns (bool);
+}
+
+contract BorrowerOperationsCrossReentrancy {
+    IBorrowerOperations public borrowerOperations;
+
+    constructor(
+        IBorrowerOperations _borrowerOperations
+    ) public {
+        borrowerOperations = _borrowerOperations;
+    }
+
+    fallback() external payable {}
+
+    function testCrossReentrancyWithoutAffectingDebt(
+        uint256 _maxFeePercentage,
+        uint256 _ZUSDAmount,
+        address _upperHint,
+        address _lowerHint,
+        address _priceFeed
+    ) public payable {
+        borrowerOperations.openTrove{value: msg.value / 2}(
+            _maxFeePercentage,
+            _ZUSDAmount,
+            _upperHint,
+            _lowerHint
+        );
+
+        // manipulate the price so that the recovery mode will be triggered
+        IPriceFeedTestnet(_priceFeed).setPrice(1e8);
+
+        // // should not revert because it's not affecting the debt
+        borrowerOperations.addColl{value: msg.value / 2}(_upperHint, _lowerHint);
+    }
+
+    /**
+    * @dev open trove and decrease the debt in the same block should revert due to reentrancy
+    */
+    function testCrossReentrancyAffectingDebt(
+        uint256 _maxFeePercentage,
+        uint256 _ZUSDAmount,
+        address _upperHint,
+        address _lowerHint,
+        address _priceFeed
+    ) public payable {
+        borrowerOperations.openTrove{value: msg.value}(
+            _maxFeePercentage,
+            _ZUSDAmount,
+            _upperHint,
+            _lowerHint
+        );
+
+        // manipulate the price so that the recovery mode will be triggered
+        IPriceFeedTestnet(_priceFeed).setPrice(1e8);
+
+        // repayZusd will affect(decrease) the debt, should revert due to reentrancy violation
+        borrowerOperations.repayZUSD(_ZUSDAmount, _upperHint, _lowerHint);
+    }
+
+    /**
+    * @dev increase the debt, then decrease the debt in the same block should revert due to reentrancy
+    * @dev the trove will be opened in the past block
+    */
+    function testCrossReentrancyByIncreasingDebt(
+        uint256 _maxFeePercentage,
+        uint256 _ZUSDAmount,
+        address _upperHint,
+        address _lowerHint,
+        address _priceFeed
+    ) public payable {
+        // manipulate the price so that the recovery mode will be triggered
+        IPriceFeedTestnet(_priceFeed).setPrice(1e17);
+        // add coll to the trove so that the TCR will be above CCR
+        borrowerOperations.addColl{value: msg.value}(_upperHint, _lowerHint);
+
+        // withdraw ZUSD to increase the debt
+        borrowerOperations.withdrawZUSD(_maxFeePercentage, _ZUSDAmount, _upperHint, _lowerHint);
+
+        // repayZusd will affect(decrease) the debt, should revert due to reentrancy violation
+        borrowerOperations.repayZUSD(_ZUSDAmount, _upperHint, _lowerHint);
+    }
+}

package.json

@@ -25,7 +25,7 @@
     "prepare:set-version": "node scripts/set-version.js",
     "prepare:dist": "yarn prepare-export-deployments && yarn prepare:set-version && yarn prepare-dist && yarn prepare-artifacts",
     "prepublishOnly": "yarn docgen",
-    "postinstall": "patch-package",
+    "postinstall": "yarn patch-package",
     "prepare-export-deployments": "yarn hardhat export --export rskSovrynMainnetDeployments.json --network rskSovrynMainnet && yarn hardhat export --export rskSovrynTestnetDeployments.json --network rskSovrynTestnet",
     "prepare-dist": "rm -rf ./dist && mkdir -p dist/contracts && rsync -a -m --exclude 'ZERO' --exclude 'TestContracts' --exclude '*/*ZERO*.sol' --exclude '*/*Community*' --exclude '*/permit2/' --include '*/' --include '*.sol' --exclude '*' 'contracts/' 'dist/contracts' && cp -R LICENSE docs 'artifacts/version' ./dist && mkdir -p dist/deployment && cp -R 'deployment/deployments/rskSovrynMainnet' 'deployment/deployments/rskSovrynTestnet' dist/deployment && cp ./README.md ./dist/README.md && mv rskSovrynTestnetDeployments.json rskSovrynMainnetDeployments.json ./dist/deployment && cp ./package.dist.json ./dist/package.json && cp ./index.dist.js ./dist/index.js",
     "dist-pack": "cd ./dist && yarn pack --filename ../sovryn-zero-contracts-package.tgz",
@@ -66,7 +66,7 @@
   "devDependencies": {
     "@ethersproject/abi": "^5.7.0",
     "@ethersproject/providers": "^5.7.2",
-    "@nomicfoundation/hardhat-chai-matchers": "^2.0.2",
+    "@nomicfoundation/hardhat-chai-matchers": "^2.0.2", 
     "@nomicfoundation/hardhat-network-helpers": "^1.0.9",
     "@nomicfoundation/hardhat-toolbox": "^3.0.0",
     "@nomicfoundation/hardhat-verify": "^1.1.1",
@@ -106,4 +106,4 @@
     "typescript": "^4.9.5",
     "web3": "^1.3.4"
   }
-}
+}

tests/js/BorrowerOperationsTest.js

@@ -10,6 +10,7 @@ const TroveManagerTester = artifacts.require("TroveManagerTester");
 const ZUSDTokenTester = artifacts.require("./ZUSDTokenTester");
 const MassetManagerTester = artifacts.require("MassetManagerTester");
 const NueMockToken = artifacts.require("NueMockToken");
+const BorrowerOperationsCrossReentrancy = artifacts.require("BorrowerOperationsCrossReentrancy");
 const { AllowanceProvider, PermitTransferFrom, SignatureTransfer } = require("@uniswap/permit2-sdk");
 
 const th = testHelpers.TestHelper;
@@ -6995,6 +6996,104 @@ contract("BorrowerOperations", async accounts => {
 
         assert.isTrue(newTCR.eq(expectedTCR));
       });
+
+      it("openTrove(): open a Trove, then withdraw collateral will not revert because it's not affecting the debt", async () => {
+        await openTrove({ ICR: toBN(dec(2, 18)), extraParams: { from: alice } });
+        await priceFeed.setPrice("105000000000000000000");
+
+        assert.isTrue(await th.checkRecoveryMode(contracts));
+
+        const extraZUSDAmount = toBN(dec(1000, 18));
+        const MIN_DEBT = (
+          await th.getNetBorrowingAmount(contracts, await contracts.borrowerOperations.MIN_NET_DEBT())
+        ).add(th.toBN(1)); // add 1 to avoid rounding issues
+        const zusdAmount = MIN_DEBT.add(extraZUSDAmount);
+
+        const price = await contracts.priceFeedTestnet.getPrice();
+        const totalDebt = await th.getOpenTroveTotalDebt(contracts, zusdAmount);
+        const ICR = toBN(dec(2, 18));
+        const borrowerOperationsCrossReentrancy = await BorrowerOperationsCrossReentrancy.new(contracts.borrowerOperations.address);
+
+        await borrowerOperationsCrossReentrancy.testCrossReentrancyWithoutAffectingDebt(
+          th._100pct,
+          extraZUSDAmount,
+          whale,
+          whale,
+          priceFeed.address,
+          {value: ICR.mul(totalDebt).div(price).mul(toBN(2))}
+        );
+      });
+
+      it("openTrove(): open a Trove, then adjust it in the same block should revert due to reentrancy", async () => {
+        await openTrove({ ICR: toBN(dec(2, 18)), extraParams: { from: alice } });
+        await priceFeed.setPrice("105000000000000000000");
+
+        assert.isTrue(await th.checkRecoveryMode(contracts));
+
+        const extraZUSDAmount = toBN(dec(1000, 18));
+        const MIN_DEBT = (
+          await th.getNetBorrowingAmount(contracts, await contracts.borrowerOperations.MIN_NET_DEBT())
+        ).add(th.toBN(1)); // add 1 to avoid rounding issues
+        const zusdAmount = MIN_DEBT.add(extraZUSDAmount);
+
+        const price = await contracts.priceFeedTestnet.getPrice();
+        const totalDebt = await th.getOpenTroveTotalDebt(contracts, zusdAmount);
+        const ICR = toBN(dec(2, 18));
+        const borrowerOperationsCrossReentrancy = await BorrowerOperationsCrossReentrancy.new(contracts.borrowerOperations.address);
+
+        await assertRevert(
+          borrowerOperationsCrossReentrancy.testCrossReentrancyAffectingDebt(
+            th._100pct,
+            extraZUSDAmount,
+            whale,
+            whale,
+            priceFeed.address,
+            {value: ICR.mul(totalDebt).div(price).mul(toBN(2))}),
+          "Recovery mode mutex locked. Try in another block"
+        );
+      });
+
+      it("open trove, increase the debt, then decrease the debt in the same block should revert due to reentrancy", async () => {
+        await openTrove({ ICR: toBN(dec(2, 18)), extraParams: { from: alice } });
+        await priceFeed.setPrice("105000000000000000000");
+        assert.isTrue(await th.checkRecoveryMode(contracts));
+
+        const extraZUSDAmount = toBN(dec(1000, 18));
+        const MIN_DEBT = (
+          await th.getNetBorrowingAmount(contracts, await contracts.borrowerOperations.MIN_NET_DEBT())
+        ).add(th.toBN(1)); // add 1 to avoid rounding issues
+        const zusdAmount = MIN_DEBT.add(extraZUSDAmount);
+
+        const price = await contracts.priceFeedTestnet.getPrice();
+        const totalDebt = await th.getOpenTroveTotalDebt(contracts, zusdAmount);
+        const ICR = toBN(dec(2, 18));
+
+        const borrowerOperationsCrossReentrancy = await BorrowerOperationsCrossReentrancy.new(contracts.borrowerOperations.address);
+        // Fund the contract with ETH for gas
+        await hre.network.provider.send("hardhat_setBalance", [
+          borrowerOperationsCrossReentrancy.address,
+          "0x56BC75E2D63100000", // 100 ETH in hex
+        ]);
+        // Impersonate the borrower test contract
+        await hre.network.provider.request({
+          method: "hardhat_impersonateAccount",
+          params: [borrowerOperationsCrossReentrancy.address],
+        });
+
+        await openTrove({ maxFeePercentage: th._100pct, extraZUSDAmount, whale, whale, ICR, extraParams: { value: ICR.mul(totalDebt).div(price).mul(toBN(10)), from: borrowerOperationsCrossReentrancy.address } });
+        await priceFeed.setPrice("10000000000000000000");
+
+        const ZUSDwithdrawal = 1; // withdraw 1 wei ZUSD
+        const collateralAmount = dec(20000, 'ether'); // Increase collateral
+
+        await assertRevert(borrowerOperationsCrossReentrancy.testCrossReentrancyByIncreasingDebt(
+          th._100pct,
+          ZUSDwithdrawal,
+          borrowerOperationsCrossReentrancy.address,
+          borrowerOperationsCrossReentrancy.address,
+          priceFeed.address,
+          {from: whale, value: collateralAmount}), "Recovery mode mutex locked. Try in another block")
+        });
     });
 
     if (!withProxy) {