Macro audit fixes (#116)

* Fix [M-1]: Encryption key can be potentially intercepted to retrieve revealed URI

* Fix [L-2]: User must approve themselves to burn their own token

* Fix [M-2]: Callback marked as OPTIONAL does revert when not implemented in extension

* Complete fix [L-2]: User must approve themselves to burn their own token

* Fix [M-3]: ERC1155Core's mint() doesn’t follow the CEI pattern can lead to potential reentrancy

* Fix [M-4]: Extension may fail to get uninstalled

* Fix [L-1]: name and symbol properties are not correctly initiated

* Fix [M-5]: Update to ClaimCondition can cause the caller paying more than expected

* Fix [L-3]: Not including EIP712’s eip712Domain() function as a fallback function, lead to the function unreachable

* Fix [L-5]: Allowing core contracts to receive native tokens without any efficient way to take it out

* Fix [L-6]: Not following ERC4906 in BatchMetadata and DelayedRevealBatchMetadata contracts

* Fix [L-7]: tokenURI should revert for invalid tokenIds

* Fix [L-8]: _validateClaimCondition() returns an outdated condition

* Fix tests from fix for [L-7]

* Fix [L-9]: Native tokens can be locked in MintableERC20

* Fix [M-7]: supportsInterface() issues

* Fix [Q-1]: batchMint for ERC1155Core contract and safeMint for ERC721Core contract are currently not supported

* Fix [Q-2] Support for multiple requiredInterfaceIds

* Fix [Q-3]: Core contract implementations are not protected against initialization

* Fix [Q-4]: MintableERC721 should have getAllMetadataBatches function

* Fix [Q-5]: Inconsistent payable tags

* Fix [Q-6]: Nitpicks

Author: nkrishang

src/ModularCore.sol

@@ -77,8 +77,6 @@ abstract contract ModularCore is IModularCore, OwnableRoles {
                             FALLBACK FUNCTION
     //////////////////////////////////////////////////////////////*/
 
-    receive() external payable {}
-
     /// @notice Routes a call to the appropriate extension contract.
     fallback() external payable {
         // Get extension function data.
@@ -147,7 +145,7 @@ abstract contract ModularCore is IModularCore, OwnableRoles {
     }
 
     /// @notice Returns whether a given interface is implemented by the contract.
-    function supportsInterface(bytes4 interfaceId) external view virtual returns (bool) {
+    function supportsInterface(bytes4 interfaceId) public view virtual returns (bool) {
         if (interfaceId == 0xffffffff) return false;
         if (supportedInterfaceRefCounter[interfaceId] > 0) return true;
         return false;
@@ -174,9 +172,11 @@ abstract contract ModularCore is IModularCore, OwnableRoles {
         ExtensionConfig memory config = IModularExtension(_extension).getExtensionConfig();
 
         // Check: ModularCore supports interface required by extension.
-        if (config.requiredInterfaceId != bytes4(0)) {
-            if (!this.supportsInterface(config.requiredInterfaceId)) {
-                revert ExtensionInterfaceNotCompatible(config.requiredInterfaceId);
+        if (config.requiredInterfaces.length != 0) {
+            for (uint256 i = 0; i < config.requiredInterfaces.length; i++) {
+                if (!supportsInterface(config.requiredInterfaces[i])) {
+                    revert ExtensionInterfaceNotCompatible(config.requiredInterfaces[i]);
+                }
             }
         }
 
@@ -271,11 +271,7 @@ abstract contract ModularCore is IModularCore, OwnableRoles {
         }
 
         if (config.registerInstallationCallback) {
-            (bool success, bytes memory returndata) =
-                _extension.delegatecall(abi.encodeCall(IInstallationCallback.onUninstall, (_data)));
-            if (!success) {
-                _revert(returndata, CallbackExecutionReverted.selector);
-            }
+            _extension.delegatecall(abi.encodeCall(IInstallationCallback.onUninstall, (_data)));
         }
 
         emit ExtensionUninstalled(msg.sender, _extension, _extension);
@@ -307,14 +303,11 @@ abstract contract ModularCore is IModularCore, OwnableRoles {
 
         if (callbackFunction.implementation != address(0)) {
             (success, returndata) = callbackFunction.implementation.delegatecall(_abiEncodedCalldata);
-        } else {
-            if (callbackMode == CallbackMode.REQUIRED) {
-                revert CallbackFunctionRequired();
+            if (!success) {
+                _revert(returndata, CallbackExecutionReverted.selector);
             }
-        }
-
-        if (!success) {
-            _revert(returndata, CallbackExecutionReverted.selector);
+        } else if (callbackMode == CallbackMode.REQUIRED) {
+            revert CallbackFunctionRequired();
         }
     }
 

src/callback/BeforeBatchMintCallbackERC1155.sol

@@ -0,0 +1,31 @@
+// SPDX-License-Identifier: Apache 2.0
+pragma solidity ^0.8.0;
+
+contract BeforeBatchMintCallbackERC1155 {
+    /*//////////////////////////////////////////////////////////////
+                                ERRORS
+    //////////////////////////////////////////////////////////////*/
+
+    error BeforeBatchMintCallbackERC1155NotImplemented();
+
+    /*//////////////////////////////////////////////////////////////
+                            EXTERNAL FUNCTIONS
+    //////////////////////////////////////////////////////////////*/
+
+    /**
+     *  @notice The beforeBatchMintERC1155 hook that is called by a core token before minting tokens.
+     *  @param to The address to mint the token to.
+     *  @param ids The tokenIds to mint.
+     *  @param amounts The amounts of tokens to mint.
+     *  @param data ABI encoded data to pass to the beforeBatchMint hook.
+     *  @return result Abi encoded bytes result of the hook.
+     */
+    function beforeBatchMintERC1155(address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data)
+        external
+        payable
+        virtual
+        returns (bytes memory result)
+    {
+        revert BeforeBatchMintCallbackERC1155NotImplemented();
+    }
+}

src/core/token/ERC1155Core.sol

@@ -7,6 +7,7 @@ import {ERC1155} from "@solady/tokens/ERC1155.sol";
 import {ModularCore} from "../../ModularCore.sol";
 
 import {BeforeMintCallbackERC1155} from "../../callback/BeforeMintCallbackERC1155.sol";
+import {BeforeBatchMintCallbackERC1155} from "../../callback/BeforeBatchMintCallbackERC1155.sol";
 import {BeforeTransferCallbackERC1155} from "../../callback/BeforeTransferCallbackERC1155.sol";
 import {BeforeBatchTransferCallbackERC1155} from "../../callback/BeforeBatchTransferCallbackERC1155.sol";
 import {BeforeBurnCallbackERC1155} from "../../callback/BeforeBurnCallbackERC1155.sol";
@@ -19,13 +20,13 @@ contract ERC1155Core is ERC1155, ModularCore, Multicallable {
     //////////////////////////////////////////////////////////////*/
 
     /// @notice The name of the NFT collection.
-    string private _name;
+    string private name_;
 
     /// @notice The symbol of the NFT collection.
-    string private _symbol;
+    string private symbol_;
 
     /// @notice The contract metadata URI of the contract.
-    string private _contractURI;
+    string private contractURI_;
 
     /// @notice The total supply of a tokenId of the NFT collection.
     mapping(uint256 => uint256) private _totalSupply;
@@ -42,23 +43,23 @@ contract ERC1155Core is ERC1155, ModularCore, Multicallable {
     //////////////////////////////////////////////////////////////*/
 
     constructor(
-        string memory name,
-        string memory symbol,
-        string memory contractURI,
-        address owner,
-        address[] memory extensions,
-        bytes[] memory extensionInstallData
+        string memory _name,
+        string memory _symbol,
+        string memory _contractURI,
+        address _owner,
+        address[] memory _extensions,
+        bytes[] memory _extensionInstallData
     ) payable {
         // Set contract metadata
-        _name = _name;
-        _symbol = _symbol;
-        _setupContractURI(contractURI);
-        _initializeOwner(owner);
+        name_ = _name;
+        symbol_ = _symbol;
+        _setupContractURI(_contractURI);
+        _initializeOwner(_owner);
 
         // Install and initialize extensions
-        require(extensions.length == extensions.length);
-        for (uint256 i = 0; i < extensions.length; i++) {
-            _installExtension(extensions[i], extensionInstallData[i]);
+        require(_extensions.length == _extensionInstallData.length);
+        for (uint256 i = 0; i < _extensions.length; i++) {
+            _installExtension(_extensions[i], _extensionInstallData[i]);
         }
     }
 
@@ -68,20 +69,20 @@ contract ERC1155Core is ERC1155, ModularCore, Multicallable {
 
     /// @notice Returns the name of the NFT Collection.
     function name() public view returns (string memory) {
-        return _name;
+        return name_;
     }
 
     /// @notice Returns the symbol of the NFT Collection.
     function symbol() public view returns (string memory) {
-        return _symbol;
+        return symbol_;
     }
 
     /**
      *  @notice Returns the contract URI of the contract.
      *  @return uri The contract URI of the contract.
      */
     function contractURI() external view returns (string memory) {
-        return _contractURI;
+        return contractURI_;
     }
 
     /**
@@ -110,7 +111,8 @@ contract ERC1155Core is ERC1155, ModularCore, Multicallable {
         return interfaceId == 0x01ffc9a7 // ERC165 Interface ID for ERC165
             || interfaceId == 0xd9b67a26 // ERC165 Interface ID for ERC1155
             || interfaceId == 0x0e89341c // ERC165 Interface ID for ERC1155MetadataURI
-            || interfaceId == 0x2a55205a // ERC165 Interface ID for ERC-2981
+            || interfaceId == 0xe8a3d485 // ERC-7572
+            || interfaceId == 0x7f5828d0 // ERC-173
             || super.supportsInterface(interfaceId); // right-most ModularCore
     }
 
@@ -120,7 +122,7 @@ contract ERC1155Core is ERC1155, ModularCore, Multicallable {
         override
         returns (SupportedCallbackFunction[] memory supportedCallbackFunctions)
     {
-        supportedCallbackFunctions = new SupportedCallbackFunction[](6);
+        supportedCallbackFunctions = new SupportedCallbackFunction[](7);
         supportedCallbackFunctions[0] = SupportedCallbackFunction({
             selector: BeforeMintCallbackERC1155.beforeMintERC1155.selector,
             mode: CallbackMode.REQUIRED
@@ -143,6 +145,10 @@ contract ERC1155Core is ERC1155, ModularCore, Multicallable {
         });
         supportedCallbackFunctions[5] =
             SupportedCallbackFunction({selector: OnTokenURICallback.onTokenURI.selector, mode: CallbackMode.REQUIRED});
+        supportedCallbackFunctions[6] = SupportedCallbackFunction({
+            selector: BeforeBatchMintCallbackERC1155.beforeBatchMintERC1155.selector,
+            mode: CallbackMode.REQUIRED
+        });
     }
 
     /*//////////////////////////////////////////////////////////////
@@ -168,9 +174,30 @@ contract ERC1155Core is ERC1155, ModularCore, Multicallable {
      */
     function mint(address to, uint256 tokenId, uint256 value, bytes memory data) external payable {
         _beforeMint(to, tokenId, value, data);
-        _mint(to, tokenId, value, "");
 
         _totalSupply[tokenId] += value;
+        _mint(to, tokenId, value, "");
+    }
+
+    /**
+     *  @notice Batch mints tokens. Calls the beforeBatchMint hook.
+     *  @dev Reverts if beforeBatchMint hook is absent or unsuccessful.
+     *  @param to The address to mint the token to.
+     *  @param ids The tokenIds to mint.
+     *  @param amounts The amounts of tokens to mint.
+     *  @param data ABI encoded data to pass to the beforeBatchMint hook.
+     */
+    function batchMint(address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data)
+        external
+        payable
+    {
+        _beforeBatchMint(to, ids, amounts, data);
+
+        for (uint256 i = 0; i < ids.length; i++) {
+            _totalSupply[ids[i]] += amounts[i];
+        }
+
+        _batchMint(to, ids, amounts, "");
     }
 
     /**
@@ -181,11 +208,11 @@ contract ERC1155Core is ERC1155, ModularCore, Multicallable {
      *  @param value The amount of tokens to burn.
      *  @param data ABI encoded data to pass to the beforeBurn hook.
      */
-    function burn(address from, uint256 tokenId, uint256 value, bytes memory data) external {
+    function burn(address from, uint256 tokenId, uint256 value, bytes memory data) external payable {
         _beforeBurn(from, tokenId, value, data);
-        _burn(msg.sender, from, tokenId, value);
 
         _totalSupply[tokenId] -= value;
+        _burn(msg.sender, from, tokenId, value);
     }
 
     /**
@@ -239,8 +266,8 @@ contract ERC1155Core is ERC1155, ModularCore, Multicallable {
     //////////////////////////////////////////////////////////////*/
 
     /// @dev Sets contract URI
-    function _setupContractURI(string memory uri) internal {
-        _contractURI = uri;
+    function _setupContractURI(string memory _contractURI) internal {
+        contractURI_ = _contractURI;
         emit ContractURIUpdated();
     }
 
@@ -256,6 +283,17 @@ contract ERC1155Core is ERC1155, ModularCore, Multicallable {
         );
     }
 
+    /// @dev Calls the beforeBatchMint hook.
+    function _beforeBatchMint(address to, uint256[] memory ids, uint256[] memory amounts, bytes memory data)
+        internal
+        virtual
+    {
+        _executeCallbackFunction(
+            BeforeBatchMintCallbackERC1155.beforeBatchMintERC1155.selector,
+            abi.encodeCall(BeforeBatchMintCallbackERC1155.beforeBatchMintERC1155, (to, ids, amounts, data))
+        );
+    }
+
     /// @dev Calls the beforeTransfer hook, if installed.
     function _beforeTransfer(address from, address to, uint256 tokenId, uint256 value) internal virtual {
         _executeCallbackFunction(