Transpile 138d094

CHANGELOG.md

@@ -1,5 +1,10 @@
 # Changelog
 
+## 4.7.1 (2022-07-19)
+
+ * `SignatureChecker`: Fix an issue that causes `isValidSignatureNow` to revert when the target contract returns ill-encoded data. ([#3552](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3552))
+ * `ERC165Checker`: Fix an issue that causes `supportsInterface` to revert when the target contract returns ill-encoded data. ([#3552](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3552))
+
 ## 4.7.0 (2022-06-29)
 
  * `TimelockController`: Migrate `_call` to `_execute` and allow inheritance and overriding similar to `Governor`. ([#3317](https://github.com/OpenZeppelin/openzeppelin-contracts/pull/3317))

contracts/mocks/ERC1271WalletMockUpgradeable.sol

@@ -28,3 +28,24 @@ contract ERC1271WalletMockUpgradeable is Initializable, OwnableUpgradeable, IERC
      */
     uint256[50] private __gap;
 }
+
+contract ERC1271MaliciousMockUpgradeable is Initializable, IERC1271Upgradeable {
+    function __ERC1271MaliciousMock_init() internal onlyInitializing {
+    }
+
+    function __ERC1271MaliciousMock_init_unchained() internal onlyInitializing {
+    }
+    function isValidSignature(bytes32, bytes memory) public pure override returns (bytes4) {
+        assembly {
+            mstore(0, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
+            return(0, 32)
+        }
+    }
+
+    /**
+     * @dev This empty reserved space is put in place to allow future versions to add new
+     * variables without shifting down storage in the inheritance chain.
+     * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
+     */
+    uint256[50] private __gap;
+}

contracts/mocks/ERC165/ERC165MaliciousDataUpgradeable.sol

@@ -0,0 +1,25 @@
+// SPDX-License-Identifier: MIT
+
+pragma solidity ^0.8.0;
+import "../../proxy/utils/Initializable.sol";
+
+contract ERC165MaliciousDataUpgradeable is Initializable {
+    function __ERC165MaliciousData_init() internal onlyInitializing {
+    }
+
+    function __ERC165MaliciousData_init_unchained() internal onlyInitializing {
+    }
+    function supportsInterface(bytes4) public view returns (bool) {
+        assembly {
+            mstore(0, 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff)
+            return(0, 32)
+        }
+    }
+
+    /**
+     * @dev This empty reserved space is put in place to allow future versions to add new
+     * variables without shifting down storage in the inheritance chain.
+     * See https://docs.openzeppelin.com/contracts/4.x/upgradeable#storage_gaps
+     */
+    uint256[50] private __gap;
+}

contracts/mocks/WithInit.sol

@@ -132,6 +132,13 @@ contract ERC1271WalletMockUpgradeableWithInit is ERC1271WalletMockUpgradeable {
         __ERC1271WalletMock_init(originalOwner);
     }
 }
+import "./ERC1271WalletMockUpgradeable.sol";
+
+contract ERC1271MaliciousMockUpgradeableWithInit is ERC1271MaliciousMockUpgradeable {
+    constructor() payable initializer {
+        __ERC1271MaliciousMock_init();
+    }
+}
 import "./SignatureCheckerMockUpgradeable.sol";
 
 contract SignatureCheckerMockUpgradeableWithInit is SignatureCheckerMockUpgradeable {
@@ -1079,6 +1086,13 @@ contract DummyImplementationV2UpgradeableWithInit is DummyImplementationV2Upgrad
         __DummyImplementationV2_init();
     }
 }
+import "./ERC165/ERC165MaliciousDataUpgradeable.sol";
+
+contract ERC165MaliciousDataUpgradeableWithInit is ERC165MaliciousDataUpgradeable {
+    constructor() payable initializer {
+        __ERC165MaliciousData_init();
+    }
+}
 import "./ERC165/ERC165MissingDataUpgradeable.sol";
 
 contract ERC165MissingDataUpgradeableWithInit is ERC165MissingDataUpgradeable {

contracts/package.json

@@ -1,7 +1,7 @@
 {
   "name": "@openzeppelin/contracts-upgradeable",
   "description": "Secure Smart Contract library for Solidity",
-  "version": "4.7.0",
+  "version": "4.7.1",
   "files": [
     "**/*.sol",
     "/build/contracts/*.json",

contracts/utils/cryptography/SignatureCheckerUpgradeable.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-// OpenZeppelin Contracts (last updated v4.5.0) (utils/cryptography/SignatureChecker.sol)
+// OpenZeppelin Contracts (last updated v4.7.1) (utils/cryptography/SignatureChecker.sol)
 
 pragma solidity ^0.8.0;
 
@@ -35,6 +35,8 @@ library SignatureCheckerUpgradeable {
         (bool success, bytes memory result) = signer.staticcall(
             abi.encodeWithSelector(IERC1271Upgradeable.isValidSignature.selector, hash, signature)
         );
-        return (success && result.length == 32 && abi.decode(result, (bytes4)) == IERC1271Upgradeable.isValidSignature.selector);
+        return (success &&
+            result.length == 32 &&
+            abi.decode(result, (bytes32)) == bytes32(IERC1271Upgradeable.isValidSignature.selector));
     }
 }

contracts/utils/introspection/ERC165CheckerUpgradeable.sol

@@ -1,5 +1,5 @@
 // SPDX-License-Identifier: MIT
-// OpenZeppelin Contracts v4.4.1 (utils/introspection/ERC165Checker.sol)
+// OpenZeppelin Contracts (last updated v4.7.1) (utils/introspection/ERC165Checker.sol)
 
 pragma solidity ^0.8.0;
 
@@ -108,6 +108,6 @@ library ERC165CheckerUpgradeable {
         bytes memory encodedParams = abi.encodeWithSelector(IERC165Upgradeable.supportsInterface.selector, interfaceId);
         (bool success, bytes memory result) = account.staticcall{gas: 30000}(encodedParams);
         if (result.length < 32) return false;
-        return success && abi.decode(result, (bool));
+        return success && abi.decode(result, (uint256)) > 0;
     }
 }

package.json

@@ -2,7 +2,7 @@
   "private": true,
   "name": "openzeppelin-solidity",
   "description": "Secure Smart Contract library for Solidity",
-  "version": "4.7.0",
+  "version": "4.7.1",
   "files": [
     "/contracts/**/*.sol",
     "/build/contracts/*.json",

test/utils/cryptography/SignatureChecker.test.js

@@ -4,6 +4,7 @@ const { expect } = require('chai');
 
 const SignatureCheckerMock = artifacts.require('SignatureCheckerMock');
 const ERC1271WalletMock = artifacts.require('ERC1271WalletMock');
+const ERC1271MaliciousMock = artifacts.require('ERC1271MaliciousMock');
 
 const TEST_MESSAGE = web3.utils.sha3('OpenZeppelin');
 const WRONG_MESSAGE = web3.utils.sha3('Nope');
@@ -14,6 +15,7 @@ contract('SignatureChecker (ERC1271)', function (accounts) {
   before('deploying', async function () {
     this.signaturechecker = await SignatureCheckerMock.new();
     this.wallet = await ERC1271WalletMock.new(signer);
+    this.malicious = await ERC1271MaliciousMock.new();
     this.signature = await web3.eth.sign(TEST_MESSAGE, signer);
   });
 
@@ -67,5 +69,13 @@ contract('SignatureChecker (ERC1271)', function (accounts) {
         this.signature,
       )).to.equal(false);
     });
+
+    it('with malicious wallet', async function () {
+      expect(await this.signaturechecker.isValidSignatureNow(
+        this.malicious.address,
+        toEthSignedMessageHash(TEST_MESSAGE),
+        this.signature,
+      )).to.equal(false);
+    });
   });
 });

test/utils/introspection/ERC165Checker.test.js

@@ -4,6 +4,7 @@ const { expect } = require('chai');
 
 const ERC165CheckerMock = artifacts.require('ERC165CheckerMock');
 const ERC165MissingData = artifacts.require('ERC165MissingData');
+const ERC165MaliciousData = artifacts.require('ERC165MaliciousData');
 const ERC165NotSupported = artifacts.require('ERC165NotSupported');
 const ERC165InterfacesSupported = artifacts.require('ERC165InterfacesSupported');
 
@@ -46,6 +47,38 @@ contract('ERC165Checker', function (accounts) {
     });
   });
 
+  context('ERC165 malicious return data', function () {
+    beforeEach(async function () {
+      this.target = await ERC165MaliciousData.new();
+    });
+
+    it('does not support ERC165', async function () {
+      const supported = await this.mock.supportsERC165(this.target.address);
+      expect(supported).to.equal(false);
+    });
+
+    it('does not support mock interface via supportsInterface', async function () {
+      const supported = await this.mock.supportsInterface(this.target.address, DUMMY_ID);
+      expect(supported).to.equal(false);
+    });
+
+    it('does not support mock interface via supportsAllInterfaces', async function () {
+      const supported = await this.mock.supportsAllInterfaces(this.target.address, [DUMMY_ID]);
+      expect(supported).to.equal(false);
+    });
+
+    it('does not support mock interface via getSupportedInterfaces', async function () {
+      const supported = await this.mock.getSupportedInterfaces(this.target.address, [DUMMY_ID]);
+      expect(supported.length).to.equal(1);
+      expect(supported[0]).to.equal(false);
+    });
+
+    it('does not support mock interface via supportsERC165InterfaceUnchecked', async function () {
+      const supported = await this.mock.supportsERC165InterfaceUnchecked(this.target.address, DUMMY_ID);
+      expect(supported).to.equal(true);
+    });
+  });
+
   context('ERC165 not supported', function () {
     beforeEach(async function () {
       this.target = await ERC165NotSupported.new();