Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Catalyst audit: N-05 _disableInitializers() Not Called in Multiple Initializable Contract Constructors #1121

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
71 commits
Select commit Hold shift + click to select a range
075ce61
feat : added events after sensitive changes
Aug 28, 2023
e6fd61f
fix : added '_disableInitializers()' in constructor
Aug 29, 2023
0418b69
fix: fixed merge conflict
Sep 7, 2023
f333356
feat : added events after sensitive changes
Aug 28, 2023
7a790c8
fix : updated missing docstring
Aug 28, 2023
d2c1701
fix : added '_disableInitializers()' in constructor
Aug 29, 2023
7281bcb
fix : updated incomplete docstrings
Aug 29, 2023
d14e82f
fix: removed duplicate constructor
Aug 31, 2023
9f119ea
feat : added events after sensitive changes
Aug 28, 2023
6c6aed4
fix : updated missing docstring
Aug 28, 2023
63be1a8
fix : added named return values
Aug 30, 2023
b28c1f1
refactor : enhance function with named return values
Aug 30, 2023
37c0e45
fix: updated contracts and interfaces
Aug 31, 2023
ffda588
fix: format fixed
Aug 31, 2023
b385c33
return the values directly
wojciech-turek Sep 1, 2023
63a5fe0
feat : added events after sensitive changes
Aug 28, 2023
1532cea
fix : updated missing docstring
Aug 28, 2023
4bd0fa2
refactor : enhance function with named return values
Aug 30, 2023
5595489
return the values directly
wojciech-turek Sep 1, 2023
899aea3
refactor : variables names
Aug 29, 2023
c810488
fix : added '_disableInitializers()' in constructor
Aug 29, 2023
a4749ac
fix : updated incomplete docstrings
Aug 29, 2023
12672d6
fix : added indexed params in events
Aug 30, 2023
6ae72f9
fix: removed duplicate constructor
Aug 31, 2023
85979b2
feat : added events after sensitive changes
Aug 28, 2023
d7b6d5e
fix : updated missing docstring
Aug 28, 2023
de639e8
refactor : enhance function with named return values
Aug 30, 2023
a09b5f3
return the values directly
wojciech-turek Sep 1, 2023
608ded4
fix : added missing parent initializer calls in contract
Aug 30, 2023
19060f8
feat : added events after sensitive changes
Aug 28, 2023
0a7ad20
fix : updated missing docstring
Aug 28, 2023
18a0221
refactor : enhance function with named return values
Aug 30, 2023
ad384de
return the values directly
wojciech-turek Sep 1, 2023
444b4f0
fix : updated functions visibility to external
Aug 30, 2023
301ca73
fix: function visibility
Sep 1, 2023
66dd01b
fix: updated function visibility
Sep 8, 2023
3a7921f
feat : added events after sensitive changes
Aug 28, 2023
5aa7c28
fix : updated missing docstring
Aug 28, 2023
a59b101
refactor : enhance function with named return values
Aug 30, 2023
2d6acf2
return the values directly
wojciech-turek Sep 1, 2023
3ccafc5
fix : fixed redundant code
Aug 30, 2023
c61fedf
fix: removed redundent code
Sep 1, 2023
bacfb0b
refactor : enhance function with named return values
Aug 30, 2023
9ddbacc
return the values directly
wojciech-turek Sep 1, 2023
f19841a
refactor : enhance function with named return values
Aug 30, 2023
ccc6408
return the values directly
wojciech-turek Sep 1, 2023
db219fe
fix : fixed typographical errors
Aug 30, 2023
e14cfa4
Fix typographical errors
wojciech-turek Sep 1, 2023
b95984a
refactor : enhance function with named return values
Aug 30, 2023
d5d799a
return the values directly
wojciech-turek Sep 1, 2023
de75a07
refactor : enhance function with named return values
Aug 30, 2023
a822c01
return the values directly
wojciech-turek Sep 1, 2023
bd489c0
refactor : enhance function with named return values
Aug 30, 2023
1eb60f2
return the values directly
wojciech-turek Sep 1, 2023
74cdcd8
refactor : used named return royalBps variable
Aug 30, 2023
26a6a93
fix: added missing function
Sep 1, 2023
920c1e3
fix: format:fix
Sep 1, 2023
129a667
refactor : enhance function with named return values
Aug 30, 2023
b87dbe7
return the values directly
wojciech-turek Sep 1, 2023
925452a
fix: added missing function
Sep 1, 2023
a594e5f
fix : removed unused variables
Aug 30, 2023
2cb5258
fix: fixed format
Sep 7, 2023
80d6ba2
refactor: removed royaltyBps value allocation
Sep 8, 2023
fdfdd31
Merge pull request #1130 from thesandboxgame/catalyst-audit/N-18-unus…
rishabh0x00 Sep 11, 2023
711d9e0
Merge pull request #1128 from thesandboxgame/catalyst-audit/N-14-typo…
rishabh0x00 Sep 11, 2023
61067e9
Merge pull request #1127 from thesandboxgame/catalyst-audit/N-12-redu…
rishabh0x00 Sep 11, 2023
bf2172b
Merge pull request #1126 from thesandboxgame/catalyst-audit/N-11-publ…
rishabh0x00 Sep 11, 2023
083d8fd
Merge pull request #1125 from thesandboxgame/catalyst-audit/N-10-miss…
rishabh0x00 Sep 11, 2023
943a005
Merge pull request #1124 from thesandboxgame/catalyst-audit/N-09-lack…
rishabh0x00 Sep 11, 2023
1273521
Merge pull request #1123 from thesandboxgame/catalyst-audit/N-08-inco…
rishabh0x00 Sep 11, 2023
15b07e1
Merge pull request #1122 from thesandboxgame/catalyst-audit/N-06-inco…
rishabh0x00 Sep 11, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/asset/contracts/Asset.sol
Original file line number Diff line number Diff line change
Expand Up @@ -336,7 +336,7 @@ contract Asset is
return TokenIdUtils.isBridged(tokenId);
}

/// @notice This function is used to register Asset contract on the Operator Filterer Registry of Opensea.can only be called by admin.
/// @notice This function is used to register Asset contract on the Operator Filterer Registry of OpenSea.can only be called by admin.
/// @dev used to register contract and subscribe to the subscriptionOrRegistrantToCopy's black list.
/// @param subscriptionOrRegistrantToCopy registration address of the list to subscribe.
/// @param subscribe bool to signify subscription "true"" or to copy the list "false".
Expand Down
10 changes: 5 additions & 5 deletions packages/asset/contracts/Catalyst.sol
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {ICatalyst} from "./interfaces/ICatalyst.sol";

/// @title Catalyst
/// @author The Sandbox
/// @notice THis contract manages catalysts which are used to mint new assets.
/// @notice This contract manages catalysts which are used to mint new assets.
/// @dev An ERC1155 contract that manages catalysts, extends multiple OpenZeppelin contracts to
/// provide a variety of features including, AccessControl, URIStorage, Burnable and more.
/// The contract includes support for meta transactions.
Expand Down Expand Up @@ -77,7 +77,7 @@ contract Catalyst is
address _defaultMinter,
string[] memory _catalystIpfsCID,
address _royaltyManager
) public initializer {
) external initializer {
require(bytes(_baseUri).length != 0, "Catalyst: base uri can't be empty");
require(_trustedForwarder != address(0), "Catalyst: trusted forwarder can't be zero");
require(_subscription != address(0), "Catalyst: subscription can't be zero");
Expand Down Expand Up @@ -235,7 +235,7 @@ contract Catalyst is
/// @param to address to which the token will be transfered.
/// @param id the token type transfered.
/// @param value amount of token transfered.
/// @param data aditional data accompanying the transfer.
/// @param data additional data accompanying the transfer.
function safeTransferFrom(
address from,
address to,
Expand All @@ -252,7 +252,7 @@ contract Catalyst is
/// @param to address to which the token will be transfered.
/// @param ids ids of each token type transfered.
/// @param values amount of each token type transfered.
/// @param data aditional data accompanying the transfer.
/// @param data additional data accompanying the transfer.
function safeBatchTransferFrom(
address from,
address to,
Expand Down Expand Up @@ -293,7 +293,7 @@ contract Catalyst is
return super.supportsInterface(interfaceId);
}

/// @notice This function is used to register Catalyst contract on the Operator Filterer Registry of Opensea.can only be called by admin.
/// @notice This function is used to register Catalyst contract on the Operator Filterer Registry of OpenSea. Can only be called by admin.
/// @dev used to register contract and subscribe to the subscriptionOrRegistrantToCopy's black list.
/// @param subscriptionOrRegistrantToCopy registration address of the list to subscribe.
/// @param subscribe bool to signify subscription "true"" or to copy the list "false".
Expand Down
8 changes: 4 additions & 4 deletions packages/asset/docs/Asset.md
Original file line number Diff line number Diff line change
Expand Up @@ -176,14 +176,15 @@ Sets a new trusted forwarder for meta-transactions.
Parameters:

- `trustedForwarder` - The new trusted forwarder.

### setTokenRoyalties

```solidity
function setTokenRoyalties(
uint256 tokenId,
address payable recipient,
address creator
) external override onlyRole(DEFAULT_ADMIN_ROLE)
) external override onlyRole(DEFAULT_ADMIN_ROLE)
```

Sets token royalty i.e. the creator splitter address as EIP2981 royalty recipient. deploys a splitter if there is none deployed for a creator. Only admin can call it.
Expand Down Expand Up @@ -254,7 +255,6 @@ Parameters:

- `tokenId` - the id of the token.


### isBridged

```solidity
Expand All @@ -275,7 +275,7 @@ function registerAndSubscribe(address subscriptionOrRegistrantToCopy, bool subsc
onlyRole(DEFAULT_ADMIN_ROLE)
```

Used to register and subscribe on the operator filter registry of Opensea
Used to register and subscribe on the operator filter registry of OpenSea

Parameters:

Expand All @@ -286,7 +286,7 @@ Parameters:

```solidity
function setOperatorRegistry(address registry) external onlyRole(DEFAULT_ADMIN_ROLE)
```fu
```

Used to the address of the operator filter registry

Expand Down
Original file line number Diff line number Diff line change
@@ -1,27 +1,28 @@
# OperatorFilterRegistry
Opensea in an attempt to regularize market places and creator earnings deployed a registry(https://github.com/ProjectOpenSea/operator-filter-registry) and asked NFT token contract to add filter logic(https://github.com/ProjectOpenSea/operator-filter-registry/blob/main/src/OperatorFilterer.sol).
# OperatorFilterRegistry

These filter has two modifier
1st : onlyAllowedOperator
OpenSea in an attempt to regularize market places and creator earnings deployed a registry(https://github.com/ProjectOpenSea/operator-filter-registry) and asked NFT token contract to add filter logic(https://github.com/ProjectOpenSea/operator-filter-registry/blob/main/src/OperatorFilterer.sol).

These filter has two modifier
1st : onlyAllowedOperator
2nd : onlyAllowedOperatorApproval

These modifiers are added to to the transfer functions(onlyAllowedOperator) and approval function(onlyAllowedOperatorApproval) such that the when not an owner tried to transfer a Token(ex: Marketplace) or owner approves an operator(ex : Marketplace) they would be filtered on the OperatorFilterRegistry.
These modifiers are added to to the transfer functions(onlyAllowedOperator) and approval function(onlyAllowedOperatorApproval) such that the when not an owner tried to transfer a Token(ex: Marketplace) or owner approves an operator(ex : Marketplace) they would be filtered on the OperatorFilterRegistry.

If the operator or the token transfer is not approved by the registry the transaction would be reverted.

On OperatorFilterRegistry a contract owner or the contract can register and maintain there own blacklist or subscribe to other blacklists but that blacklist should contain the default marketplaces blacklisted by Opensea.
On OperatorFilterRegistry a contract owner or the contract can register and maintain there own blacklist or subscribe to other blacklists but that blacklist should contain the default marketplaces blacklisted by OpenSea.

# OperatorFiltererRegistrant
# OperatorFiltererRegistrant

The OperatorFiltererRegistrant contract is made to be registered on the OperatorFilterRegistry and copy the default blacklist of the openSea. This contract would then be subscribed by the contract such as AssetERC721 and AssetERC1155.
The OperatorFiltererRegistrant contract is made to be registered on the OperatorFilterRegistry and copy the default blacklist of the OpenSea. This contract would then be subscribed by the contract such as AssetERC721 and AssetERC1155.

The OperatorFiltererRegistrant would be the subscription for our token contracts on a layer(layer-1 : Ethereum , layer-2: Polygon), such that when a address is added or removed from OperatorFiltererRegistrant's blacklist it would be come in affect for each contact which subscribe to the OperatorFiltererRegistrant's blacklist.

# Intended usage

The OperatorFiltererRegistrant is so that sandbox will have a common blacklist that would be utilized by every Token contract on a layer. This would create a single list that would be subscribed by each contract to provide uniformity to which market places sandbox wants to blacklist. This would also provide a focal point to remove and add market places such that it would be applicable to every contract that subscribe to it.

# Implementation
# Implementation

We didn't use the npm package as its solidity pragma(solidity version) doesn't match the one we have for our Asset contracts and updating our solidity version for Assets would have been to time consuming.

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ import {Ownable} from "@openzeppelin/contracts/access/Ownable.sol";

/// @title OperatorFilterSubription
/// @author The Sandbox
/// @notice This contract is meant to register and copy the default subscription of the OpenSea for the operator filter and our Token contract are supposed to subscribe to this contract on openSea operator filter registry
/// @notice This contract is meant to register and copy the default subscription of the OpenSea for the operator filter and our Token contract are supposed to subscribe to this contract on OpenSea operator filter registry
contract OperatorFilterSubscription is Ownable {
address public constant DEFAULT_SUBSCRIPTION = address(0x3cc6CddA760b79bAfa08dF41ECFA224f810dCeB6);
address public constant DEFAULT_SUBSCRIPTION = 0x3cc6CddA760b79bAfa08dF41ECFA224f810dCeB6;

// solhint-disable-next-line const-name-snakecase
IOperatorFilterRegistry public constant OPERATOR_FILTER_REGISTRY =
IOperatorFilterRegistry(0x000000000000AAeB6D7670E522A718067333cd4E);

constructor() Ownable() {
// Subscribe and copy the entries of the Default subscription list of open sea.
// Subscribe and copy the entries of the Default subscription list of OpenSea.
if (address(OPERATOR_FILTER_REGISTRY).code.length > 0) {
OPERATOR_FILTER_REGISTRY.registerAndCopyEntries(address(this), DEFAULT_SUBSCRIPTION);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import {IOperatorFilterRegistry} from "./interfaces/IOperatorFilterRegistry.sol"
import {ContextUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/ContextUpgradeable.sol";

///@title OperatorFiltererUpgradeable
///@author The SandBox
///@notice This contract would subscibe or copy or just to the subscription provided or just register to default subscription list. The operator filter registry's addess could be set using a setter which could be implemented in inherting contract
///@author The Sandbox
///@notice This contract would subscribe or copy or just to the subscription provided or just register to default subscription list. The operator filter registry's address could be set using a setter which could be implemented in inheriting contract
abstract contract OperatorFiltererUpgradeable is Initializable, ContextUpgradeable {
event OperatorFilterRegistrySet(address indexed registry);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ pragma solidity ^0.8.0;
interface IOperatorFilterRegistry {
///@notice Returns true if operator is not filtered for a given token, either by address or codeHash. Also returns
/// true if supplied registrant address is not registered.
function isOperatorAllowed(address registrant, address operator) external view returns (bool);
function isOperatorAllowed(address registrant, address operator) external view returns (bool isAllowed);

///@notice Registers an address with the registry. May be called by address itself or by EIP-173 owner.
function register(address registrant) external;
Expand Down Expand Up @@ -66,46 +66,44 @@ interface IOperatorFilterRegistry {

///@notice Get the set of addresses subscribed to a given registrant.
/// Note that order is not guaranteed as updates are made.

function subscribers(address registrant) external returns (address[] memory);
function subscribers(address registrant) external returns (address[] memory subscribersList);

///@notice Get the subscriber at a given index in the set of addresses subscribed to a given registrant.
/// Note that order is not guaranteed as updates are made.
function subscriberAt(address registrant, uint256 index) external returns (address);
function subscriberAt(address registrant, uint256 index) external returns (address subscriberAddress);

///@notice Copy filtered operators and codeHashes from a different registrantToCopy to addr.

function copyEntriesOf(address registrant, address registrantToCopy) external;

///@notice Returns true if operator is filtered by a given address or its subscription.
function isOperatorFiltered(address registrant, address operator) external returns (bool);
function isOperatorFiltered(address registrant, address operator) external returns (bool isFiltered);

///@notice Returns true if the hash of an address's code is filtered by a given address or its subscription.
function isCodeHashOfFiltered(address registrant, address operatorWithCode) external returns (bool);
function isCodeHashOfFiltered(address registrant, address operatorWithCode) external returns (bool isFiltered);

///@notice Returns true if a codeHash is filtered by a given address or its subscription.
function isCodeHashFiltered(address registrant, bytes32 codeHash) external returns (bool);
function isCodeHashFiltered(address registrant, bytes32 codeHash) external returns (bool isFiltered);

///@notice Returns a list of filtered operators for a given address or its subscription.
function filteredOperators(address addr) external returns (address[] memory);
function filteredOperators(address addr) external returns (address[] memory operatorList);

///@notice Returns the set of filtered codeHashes for a given address or its subscription.
/// Note that order is not guaranteed as updates are made.
function filteredCodeHashes(address addr) external returns (bytes32[] memory);
function filteredCodeHashes(address addr) external returns (bytes32[] memory codeHashList);

///@notice Returns the filtered operator at the given index of the set of filtered operators for a given address or
/// its subscription.
/// Note that order is not guaranteed as updates are made.
function filteredOperatorAt(address registrant, uint256 index) external returns (address);
function filteredOperatorAt(address registrant, uint256 index) external returns (address operator);

///@notice Returns the filtered codeHash at the given index of the list of filtered codeHashes for a given address or
/// its subscription.
/// Note that order is not guaranteed as updates are made.
function filteredCodeHashAt(address registrant, uint256 index) external returns (bytes32);
function filteredCodeHashAt(address registrant, uint256 index) external returns (bytes32 codeHash);

///@notice Returns true if an address has registered
function isRegistered(address addr) external returns (bool);
function isRegistered(address addr) external returns (bool registered);

///@dev Convenience method to compute the code hash of an arbitrary contract
function codeHashOf(address addr) external returns (bytes32);
function codeHashOf(address addr) external returns (bytes32 codeHash);
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {

/**
* @title MockOperatorFilterRegistry
* @notice Made based on the OperatorFilterRegistry of openSea at https://github.com/ProjectOpenSea/operator-filter-registry/blob/main/src/OperatorFilterRegistry.sol
* @notice Made based on the OperatorFilterRegistry of OpenSea at https://github.com/ProjectOpenSea/operator-filter-registry/blob/main/src/OperatorFilterRegistry.sol
* @notice This contracts allows tokens or token owners to register specific addresses or codeHashes that may be
* * restricted according to the isOperatorAllowed function.
*/
Expand Down
Loading