Browse Source

Merge branch 'master' into typo-fixes

Hadrien Croubois 1 month ago
parent
commit
8134a56f51

+ 5 - 0
.changeset/quick-pianos-press.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`ReentrancyGuard` and `ReentrancyGuardTransient`: Add `nonReentrantView`, a read-only version of the `nonReentrant` modifier.

+ 5 - 0
contracts/mocks/ReentrancyAttack.sol

@@ -9,4 +9,9 @@ contract ReentrancyAttack is Context {
         (bool success, ) = _msgSender().call(data);
         require(success, "ReentrancyAttack: failed call");
     }
+
+    function staticcallSender(bytes calldata data) public view {
+        (bool success, ) = _msgSender().staticcall(data);
+        require(success, "ReentrancyAttack: failed call");
+    }
 }

+ 9 - 0
contracts/mocks/ReentrancyMock.sol

@@ -16,6 +16,10 @@ contract ReentrancyMock is ReentrancyGuard {
         _count();
     }
 
+    function viewCallback() external view nonReentrantView returns (uint256) {
+        return counter;
+    }
+
     function countLocalRecursive(uint256 n) public nonReentrant {
         if (n > 0) {
             _count();
@@ -36,6 +40,11 @@ contract ReentrancyMock is ReentrancyGuard {
         attacker.callSender(abi.encodeCall(this.callback, ()));
     }
 
+    function countAndCallView(ReentrancyAttack attacker) public nonReentrant {
+        _count();
+        attacker.staticcallSender(abi.encodeCall(this.viewCallback, ()));
+    }
+
     function _count() private {
         counter += 1;
     }

+ 9 - 0
contracts/mocks/ReentrancyTransientMock.sol

@@ -16,6 +16,10 @@ contract ReentrancyTransientMock is ReentrancyGuardTransient {
         _count();
     }
 
+    function viewCallback() external view nonReentrantView returns (uint256) {
+        return counter;
+    }
+
     function countLocalRecursive(uint256 n) public nonReentrant {
         if (n > 0) {
             _count();
@@ -36,6 +40,11 @@ contract ReentrancyTransientMock is ReentrancyGuardTransient {
         attacker.callSender(abi.encodeCall(this.callback, ()));
     }
 
+    function countAndCallView(ReentrancyAttack attacker) public nonReentrant {
+        _count();
+        attacker.staticcallSender(abi.encodeCall(this.viewCallback, ()));
+    }
+
     function _count() private {
         counter += 1;
     }

+ 19 - 2
contracts/utils/ReentrancyGuard.sol

@@ -61,11 +61,28 @@ abstract contract ReentrancyGuard {
         _nonReentrantAfter();
     }
 
-    function _nonReentrantBefore() private {
-        // On the first call to nonReentrant, _status will be NOT_ENTERED
+    /**
+     * @dev A `view` only version of {nonReentrant}. Use to block view functions
+     * from being called, preventing reading from inconsistent contract state.
+     *
+     * CAUTION: This is a "view" modifier and does not change the reentrancy
+     * status. Use it only on view functions. For payable or non-payable functions,
+     * use the standard {nonReentrant} modifier instead.
+     */
+    modifier nonReentrantView() {
+        _nonReentrantBeforeView();
+        _;
+    }
+
+    function _nonReentrantBeforeView() private view {
         if (_status == ENTERED) {
             revert ReentrancyGuardReentrantCall();
         }
+    }
+
+    function _nonReentrantBefore() private {
+        // On the first call to nonReentrant, _status will be NOT_ENTERED
+        _nonReentrantBeforeView();
 
         // Any calls to nonReentrant after this point will fail
         _status = ENTERED;

+ 19 - 2
contracts/utils/ReentrancyGuardTransient.sol

@@ -37,11 +37,28 @@ abstract contract ReentrancyGuardTransient {
         _nonReentrantAfter();
     }
 
-    function _nonReentrantBefore() private {
-        // On the first call to nonReentrant, REENTRANCY_GUARD_STORAGE.asBoolean().tload() will be false
+    /**
+     * @dev A `view` only version of {nonReentrant}. Use to block view functions
+     * from being called, preventing reading from inconsistent contract state.
+     *
+     * CAUTION: This is a "view" modifier and does not change the reentrancy
+     * status. Use it only on view functions. For payable or non-payable functions,
+     * use the standard {nonReentrant} modifier instead.
+     */
+    modifier nonReentrantView() {
+        _nonReentrantBeforeView();
+        _;
+    }
+
+    function _nonReentrantBeforeView() private view {
         if (_reentrancyGuardEntered()) {
             revert ReentrancyGuardReentrantCall();
         }
+    }
+
+    function _nonReentrantBefore() private {
+        // On the first call to nonReentrant, REENTRANCY_GUARD_STORAGE.asBoolean().tload() will be false
+        _nonReentrantBeforeView();
 
         // Any calls to nonReentrant after this point will fail
         REENTRANCY_GUARD_STORAGE.asBoolean().tstore(true);

+ 1 - 1
fv-requirements.txt

@@ -1,4 +1,4 @@
 certora-cli==8.1.1
 # File uses a custom name (fv-requirements.txt) so that it isn't picked by Netlify's build
 # whose latest Python version is 0.3.8, incompatible with most recent versions of Halmos
-halmos==0.3.1
+halmos==0.3.3

+ 12 - 5
package-lock.json

@@ -9223,17 +9223,24 @@
       "license": "ISC"
     },
     "node_modules/sha.js": {
-      "version": "2.4.11",
-      "resolved": "https://registry.npmjs.org/sha.js/-/sha.js-2.4.11.tgz",
-      "integrity": "sha512-QMEp5B7cftE7APOjk5Y6xgrbWu+WkLVQwk8JNjZ8nKRciZaByEW6MubieAiToS7+dwvrjGhH8jRXz3MVd0AYqQ==",
+      "version": "2.4.12",
+      "resolved": "https://registry.npmjs.org/sha.js/-/sha.js-2.4.12.tgz",
+      "integrity": "sha512-8LzC5+bvI45BjpfXU8V5fdU2mfeKiQe1D1gIMn7XUlF3OTUrpdJpPPH4EMAnF0DsHHdSZqCdSss5qCmJKuiO3w==",
       "dev": true,
       "license": "(MIT AND BSD-3-Clause)",
       "dependencies": {
-        "inherits": "^2.0.1",
-        "safe-buffer": "^5.0.1"
+        "inherits": "^2.0.4",
+        "safe-buffer": "^5.2.1",
+        "to-buffer": "^1.2.0"
       },
       "bin": {
         "sha.js": "bin.js"
+      },
+      "engines": {
+        "node": ">= 0.10"
+      },
+      "funding": {
+        "url": "https://github.com/sponsors/ljharb"
       }
     },
     "node_modules/sha1": {

+ 12 - 4
test/utils/ReentrancyGuard.test.js

@@ -7,7 +7,8 @@ for (const variant of ['', 'Transient']) {
     async function fixture() {
       const name = `Reentrancy${variant}Mock`;
       const mock = await ethers.deployContract(name);
-      return { name, mock };
+      const attacker = await ethers.deployContract('ReentrancyAttack');
+      return { name, mock, attacker };
     }
 
     beforeEach(async function () {
@@ -20,9 +21,16 @@ for (const variant of ['', 'Transient']) {
       expect(await this.mock.counter()).to.equal(1n);
     });
 
-    it('does not allow remote callback', async function () {
-      const attacker = await ethers.deployContract('ReentrancyAttack');
-      await expect(this.mock.countAndCall(attacker)).to.be.revertedWith('ReentrancyAttack: failed call');
+    it('nonReentrantView function can be called', async function () {
+      await this.mock.viewCallback();
+    });
+
+    it('does not allow remote callback to nonReentrant function', async function () {
+      await expect(this.mock.countAndCall(this.attacker)).to.be.revertedWith('ReentrancyAttack: failed call');
+    });
+
+    it('does not allow remote callback to nonReentrantView function', async function () {
+      await expect(this.mock.countAndCallView(this.attacker)).to.be.revertedWith('ReentrancyAttack: failed call');
     });
 
     it('_reentrancyGuardEntered should be true when guarded', async function () {