[SOV-4416] apply reentrancy guard zero

contracts/BorrowerOperations.sol

@@ -16,12 +16,14 @@ import "./Dependencies/console.sol";
 import "./BorrowerOperationsStorage.sol";
 import "./Dependencies/Mynt/MyntLib.sol";
 import "./Interfaces/IPermit2.sol";
+import "./Dependencies/reentrancy/SharedReentrancyGuard.sol";
 
 contract BorrowerOperations is
     LiquityBase,
     BorrowerOperationsStorage,
     CheckContract,
-    IBorrowerOperations
+    IBorrowerOperations,
+    SharedReentrancyGuard
 {
     /** CONSTANT / IMMUTABLE VARIABLE ONLY */
     IPermit2 public immutable permit2;
@@ -203,6 +205,8 @@ contract BorrowerOperations is
         vars.price = priceFeed.fetchPrice();
         bool isRecoveryMode = _checkRecoveryMode(vars.price);
 
+        if(isRecoveryMode) nonReentrantCheck(true);
+
         _requireValidMaxFeePercentage(_maxFeePercentage, isRecoveryMode);
         _requireTroveisNotActive(contractsCache.troveManager, msg.sender);
 
@@ -585,6 +589,10 @@ contract BorrowerOperations is
         vars.price = priceFeed.fetchPrice();
         vars.isRecoveryMode = _checkRecoveryMode(vars.price);
 
+        if(vars.isRecoveryMode) {
+            nonReentrantCheck(_isDebtIncrease);
+        }
+
         if (_isDebtIncrease) {
             _requireValidMaxFeePercentage(_maxFeePercentage, vars.isRecoveryMode);
             _requireNonZeroDebtChange(_ZUSDChange);

contracts/Dependencies/reentrancy/SharedReentrancyGuard.sol

@@ -0,0 +1,38 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.6.11;
+
+import "../../Interfaces/IZeroProtocolMutex.sol";
+
+/*
+ * @title contract for shared reentrancy guards
+ *
+ * @dev This contract exposes the modifiers for opening, closing, increasing, decreasing trove functionality
+ * The objective is we will not allow the opening, increase/decrease, closing functionalityto be executed in the same block
+ *
+ * @dev The ZeroProtocolMutex contract address is hardcoded because the address is deployed using a
+ * special deployment method (similar to ERC1820Registry). This contract therefore has no
+ * state and is thus safe to add to the inheritance chain of upgradeable contracts.
+ */
+contract SharedReentrancyGuard {
+    /*
+     * This is the address of the zero protocol mutex contract that will be used as the
+     * reentrancy guard.
+     *
+     * The address is hardcoded to avoid changing the memory layout of
+     * derived contracts (possibly upgradable). Hardcoding the address is possible,
+     * because the Mutex contract is always deployed to the same address, with the
+     * same method used in the deployment of ERC1820Registry.
+     */
+    IZeroProtocolMutex private constant MUTEX = IZeroProtocolMutex(0x42B023F998d7B9c127e9bDcDCE57ccd1f5e1d919);
+
+    /*
+     * @dev function that is responsible to handle the mutex (user's block number) check
+     * @param _isOpening flag whether it is true (opening, increasing), and false (closing, decreasing)
+     * If it is true, it will just set the user's block number to the current block number
+     * If it is false and the user's block number was not 0, it will check the user's block number to be not the same block
+     * and if the check pass, it will reset the user's block number to 0
+     */
+    function nonReentrantCheck(bool _isOpening) internal {
+        MUTEX.handleMutex(_isOpening);
+    }
+}

contracts/Dependencies/reentrancy/ZeroProtocolMutex.sol

@@ -0,0 +1,33 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.6.11;
+
+/*
+ * @title Zero Protocol Mutex contract
+ *
+ * @notice A mutex mechanism contract that will handle some function executions not to be in the same block
+ */
+contract ZeroProtocolMutex {
+    /*
+     * We use an uint to store the mutex state.
+     */
+    mapping(address => uint256) public userBlockNumber;
+
+    /*
+     * @notice set the user's block number for opening, and do check & reset to 0 for closing
+     *
+     * @dev This is the function will be called by the open, close, increase, decrease trove function
+     */
+    function handleMutex(bool _isOpening) external {
+        if(_isOpening) {
+            userBlockNumber[tx.origin] = block.number;
+        } else {
+            if(userBlockNumber[tx.origin] > 0) {
+                if(userBlockNumber[tx.origin] == block.number) {
+                    revert("ZeroProtocolMutex: mutex locked");
+                }
+
+                userBlockNumber[tx.origin] = 0;
+            }
+        }
+    }
+}

contracts/Interfaces/IZeroProtocolMutex.sol

@@ -0,0 +1,6 @@
+pragma solidity 0.6.11;
+
+interface IZeroProtocolMutex {
+    function handleMutex(bool) external;
+    function userBlockNumber(address) external view returns(uint256);
+}

contracts/TestContracts/BorrowerOperationsCrossReentrancy.sol

@@ -0,0 +1,41 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.6.11;
+
+import "../Interfaces/IBorrowerOperations.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 testCrossReentrancy(
+        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);
+
+        // // should revert due to reentrancy violation
+        borrowerOperations.addColl(_upperHint, _lowerHint);
+    }
+}

contracts/TestContracts/TestNonReentrantValueSetter.sol

@@ -0,0 +1,27 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.6.11;
+
+import "../Dependencies/reentrancy/SharedReentrancyGuard.sol";
+
+contract TestNonReentrantValueSetter is SharedReentrancyGuard {
+    uint256 public value;
+
+    function setValueOpening(uint256 newValue) public {
+        nonReentrantCheck(true);
+        value = newValue;
+    }
+
+    function setValueClosing(uint256 newValue) public {
+        nonReentrantCheck(false);
+        value = newValue;
+    }
+
+    // this will always fail
+    function setOtherContractValueNonReentrant(
+        address other,
+        uint256 newValue
+    ) external {
+        nonReentrantCheck(true);
+        TestNonReentrantValueSetter(other).setValueClosing(newValue);
+    }
+}

contracts/TestContracts/TestValueSetter.sol

@@ -0,0 +1,14 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.6.11;
+
+contract TestValueSetter {
+    uint256 public value;
+
+    function setValueOpening(uint256 newValue) public {
+        value = newValue;
+    }
+
+    function setValueClosing(uint256 newValue) public {
+        value = newValue;
+    }
+}

contracts/TestContracts/TestValueSetterProxy.sol

@@ -0,0 +1,9 @@
+// SPDX-License-Identifier: MIT
+pragma solidity 0.6.11;
+
+import "../Proxy/UpgradableProxy.sol";
+
+contract TestValueSetterProxy is UpgradableProxy {
+    // This is here for the memory layout
+    uint256 public value;
+}

deployment/deploy/7-DeployZeroProtocolMutex.ts

@@ -0,0 +1,28 @@
+const Logs = require("node-logs");
+const logger = new Logs().showInConsole(true);
+const { SAVED_DEPLOY_DATA_ZERO, getOrDeployZeroProtocolMutex, createZeroProtocolMutexDeployTransaction } = require("../helpers/reentrancy/utils");
+
+const func = async function (hre) {
+    const {
+        deployments: { deploy, log, getOrNull },
+        getNamedAccounts,
+        network,
+        ethers,
+    } = hre;
+    const { deployerAddress, contractAddress } = SAVED_DEPLOY_DATA_ZERO;
+    logger.warn("Deploying Zero Protocol Mutex...");
+
+    if (ethers.provider.getBalance(deployerAddress) === 0) {
+        throw new Error("Deployer balance is zero");
+    }
+
+    console.log(await createZeroProtocolMutexDeployTransaction());
+
+    const zeroProtocolMutex = await getOrDeployZeroProtocolMutex();
+    if (zeroProtocolMutex.target !== contractAddress) {
+        throw new Error(`Mutex address is ${zeroProtocolMutex.target}, expected ${contractAddress}`);
+    }
+    logger.warn("Zero Protocol Mutex deployed");
+};
+func.tags = ["ZeroProtocolMutex"];
+module.exports = func;

deployment/helpers/reentrancy/utils.js

@@ -0,0 +1,110 @@
+const { ethers } = require("hardhat");
+const testHelpers = require("../../../utils/js/testHelpers.js");
+const { Transaction, Wallet, Provider, getCreateAddress } = require("ethers");
+
+const th = testHelpers.TestHelper;
+const toBN = th.toBN;
+
+const SAVED_DEPLOY_DATA_ZERO = {
+    serializedDeployTx:
+        "0xf901f8808403ef14808302194d8080b901a6608060405234801561001057600080fd5b50610186806100206000396000f3fe608060405234801561001057600080fd5b50600436106100365760003560e01c80636981665b1461003b57806369b13ba014610073575b600080fd5b6100616004803603602081101561005157600080fd5b50356001600160a01b0316610094565b60408051918252519081900360200190f35b6100926004803603602081101561008957600080fd5b503515156100a6565b005b60006020819052908152604090205481565b80156100c35732600090815260208190526040902043905561014d565b326000908152602081905260409020541561014d573260009081526020819052604090205443141561013c576040805162461bcd60e51b815260206004820152601f60248201527f5a65726f50726f746f636f6c4d757465783a206d75746578206c6f636b656400604482015290519081900360640190fd5b326000908152602081905260408120555b5056fea26469706673582212208fa44190280703d199dee337f61f207770f627ddf810bdbd4a1c74fcb8b31e2664736f6c634300060b00331ba06d757465786d757465786d757465786d757465786d757465786d757465786d75a06d757465786d757465786d757465786d757465786d757465786d757465786d75",
+    deployerAddress: "0xb339D675F5Fb2EEec8a13db76dD57C11F4Af3869",
+    contractAddress: "0x42B023F998d7B9c127e9bDcDCE57ccd1f5e1d919",
+    transactionCostWei: toBN(9078234000000),
+};
+
+const getOrDeployZeroProtocolMutex = async () => {
+    const provider = ethers.provider;
+
+    const { serializedDeployTx, deployerAddress, contractAddress, transactionCostWei } =
+    SAVED_DEPLOY_DATA_ZERO;
+    const ZeroProtocolMutex = await ethers.getContractAt("ZeroProtocolMutex", contractAddress);
+    const deployedCode = await provider.getCode(contractAddress);
+    if (deployedCode.replace(/0+$/) !== "0x") {
+        // Contract is deployed
+        // it's practically impossible to deploy to this address with malicious bytecode so we don't need to check
+        return ZeroProtocolMutex;
+    }
+
+    // Not deployed, we need to deploy
+    console.log("ZeroProtocolMutex has not been deployed, deploying...")
+
+    // Fund the account
+    const deployerBalance = await provider.getBalance(deployerAddress);
+    const whale = (await ethers.getSigners())[0];
+    if (deployerBalance < transactionCostWei) {
+        const requiredBalance = toBN(transactionCostWei.toString()).sub(toBN(deployerBalance.toString()));
+        const tx = await whale.sendTransaction({
+            to: deployerAddress,
+            value: requiredBalance.toString(),
+        });
+        await tx.wait();
+    }
+
+    const tx = await provider.broadcastTransaction(serializedDeployTx);
+    await tx.wait();
+
+    return ZeroProtocolMutex.attach(contractAddress);
+};
+
+async function createZeroProtocolMutexDeployTransaction() {
+    const provider = ethers.provider;
+    const ZeroProtocolMutex = await ethers.getContractFactory("ZeroProtocolMutex");
+    const { data: bytecode } = await ZeroProtocolMutex.getDeployTransaction();
+    console.log(bytecode)
+
+    const signature = {
+        v: 27, // must not be eip-155 to allow cross-chain deployments
+        // "mutex" in hex: 6d75746578
+        //  0xm u t e x m u t e x m u t e x m u t e x m u t e x m u t e x m u
+        r: "0x6d757465786d757465786d757465786d757465786d757465786d757465786d75",
+        s: "0x6d757465786d757465786d757465786d757465786d757465786d757465786d75",
+    };
+
+    const hardhatGasLimit = await provider.estimateGas({ data: bytecode });
+    const gasLimit = toBN(137549);
+    if (hardhatGasLimit > gasLimit) {
+        throw new Error(
+            `Hardhat estimates the gas limit as ${hardhatGasLimit.toString()}, ` +
+                `which is higher than the hardcoded gas limit ${gasLimit.toString()}`
+        );
+    }
+
+    // 10 gwei, should be enough to also mine on other chains. Could also be 100 like with erc1820
+    const gasPrice = toBN(66000000);
+
+    const transactionCostWei = gasLimit.mul(gasPrice);
+
+    const deployTx = {
+        data: bytecode, // We could hardcode this too
+        nonce: 0,
+        gasLimit: gasLimit.toString(),
+        gasPrice: gasPrice.toString(),
+        type: 0
+    };
+
+    const transaction = Transaction.from({...deployTx, signature});
+    const serializedDeployTx = transaction.serialized;
+
+    console.log(transactionCostWei.toString())
+
+    const parsedDeployTx = Transaction.from(serializedDeployTx);
+    const contractAddress = await getCreateAddress({
+        from: parsedDeployTx.from,
+        nonce: parsedDeployTx.nonce,
+    });
+    const deployerAddress = parsedDeployTx.from;
+
+    return {
+        serializedDeployTx,
+        deployerAddress,
+        contractAddress,
+        transactionCostWei,
+    };
+}
+
+module.exports = {
+    getOrDeployZeroProtocolMutex,
+    createZeroProtocolMutexDeployTransaction,
+    SAVED_DEPLOY_DATA_ZERO
+};

tests/js/AccessControlTest.js

@@ -2,6 +2,7 @@ const deploymentHelper = require("../../utils/js/deploymentHelpers.js");
 const testHelpers = require("../../utils/js/testHelpers.js");
 const TroveManagerTester = artifacts.require("TroveManagerTester");
 const PriceFeedSovryn = artifacts.require("PriceFeedSovrynTester");
+const { getOrDeployZeroProtocolMutex } = require("../../deployment/helpers/reentrancy/utils");
 
 const th = testHelpers.TestHelper;
 const timeValues = testHelpers.TimeValues;
@@ -39,6 +40,8 @@ contract(
     let communityIssuance;
 
     before(async () => {
+      await getOrDeployZeroProtocolMutex();
+
       coreContracts = await deploymentHelper.deployLiquityCore();
       coreContracts.troveManager = await TroveManagerTester.new(coreContracts.permit2.address);
       coreContracts = await deploymentHelper.deployZUSDTokenTester(coreContracts);