diff --git a/contracts/access/manager/AuthorityUtils.sol b/contracts/access/manager/AuthorityUtils.sol index 61cf0407c..58962ce1a 100644 --- a/contracts/access/manager/AuthorityUtils.sol +++ b/contracts/access/manager/AuthorityUtils.sol @@ -26,6 +26,10 @@ library AuthorityUtils { if staticcall(gas(), authority, add(data, 0x20), mload(data), 0x00, 0x40) { immediate := mload(0x00) delay := mload(0x20) + + // If delay does not fit in a uint32, return 0 (no delay) + // equivalent to: if gt(delay, 0xFFFFFFFF) { delay := 0 } + delay := mul(delay, iszero(shr(32, delay))) } } } diff --git a/contracts/governance/extensions/GovernorVotesSuperQuorumFraction.sol b/contracts/governance/extensions/GovernorVotesSuperQuorumFraction.sol index 426738951..3eac72cbe 100644 --- a/contracts/governance/extensions/GovernorVotesSuperQuorumFraction.sol +++ b/contracts/governance/extensions/GovernorVotesSuperQuorumFraction.sol @@ -61,6 +61,7 @@ abstract contract GovernorVotesSuperQuorumFraction is GovernorVotesQuorumFractio /** * @dev Returns the super quorum for a `timepoint`, in terms of number of votes: `supply * numerator / denominator`. + * See {GovernorSuperQuorum-superQuorum} for more details. */ function superQuorum(uint256 timepoint) public view virtual override returns (uint256) { return Math.mulDiv(token().getPastTotalSupply(timepoint), superQuorumNumerator(timepoint), quorumDenominator()); diff --git a/contracts/interfaces/draft-IERC6909.sol b/contracts/interfaces/draft-IERC6909.sol index 12151e886..fa80dbbe3 100644 --- a/contracts/interfaces/draft-IERC6909.sol +++ b/contracts/interfaces/draft-IERC6909.sol @@ -49,7 +49,8 @@ interface IERC6909 is IERC165 { function isOperator(address owner, address spender) external view returns (bool); /** - * @dev Sets an approval to `spender` for `amount` tokens of type `id` from the caller's tokens. + * @dev Sets an approval to `spender` for `amount` of tokens of type `id` from the caller's tokens. An `amount` of + * `type(uint256).max` signifies an unlimited approval. * * Must return true. */ diff --git a/contracts/mocks/AuthorityMock.sol b/contracts/mocks/AuthorityMock.sol index 4f3e1de3a..cd2356ec1 100644 --- a/contracts/mocks/AuthorityMock.sol +++ b/contracts/mocks/AuthorityMock.sol @@ -12,7 +12,7 @@ contract NotAuthorityMock is IAuthority { } contract AuthorityNoDelayMock is IAuthority { - bool _immediate; + bool private _immediate; function canCall( address /* caller */, @@ -28,14 +28,14 @@ contract AuthorityNoDelayMock is IAuthority { } contract AuthorityDelayMock { - bool _immediate; - uint32 _delay; + bool private _immediate; + uint256 private _delay; function canCall( address /* caller */, address /* target */, bytes4 /* selector */ - ) external view returns (bool immediate, uint32 delay) { + ) external view returns (bool immediate, uint256 delay) { return (_immediate, _delay); } @@ -43,7 +43,7 @@ contract AuthorityDelayMock { _immediate = immediate; } - function _setDelay(uint32 delay) external { + function _setDelay(uint256 delay) external { _delay = delay; } } diff --git a/contracts/token/ERC6909/draft-ERC6909.sol b/contracts/token/ERC6909/draft-ERC6909.sol index e821d4b3b..702c304e7 100644 --- a/contracts/token/ERC6909/draft-ERC6909.sol +++ b/contracts/token/ERC6909/draft-ERC6909.sol @@ -79,7 +79,7 @@ contract ERC6909 is Context, ERC165, IERC6909 { /** * @dev Creates `amount` of token `id` and assigns them to `account`, by transferring it from address(0). - * Relies on the `_update` mechanism + * Relies on the `_update` mechanism. * * Emits a {Transfer} event with `from` set to the zero address. * @@ -93,10 +93,9 @@ contract ERC6909 is Context, ERC165, IERC6909 { } /** - * @dev Moves `amount` of token `id` from `from` to `to` without checking for approvals. - * - * This internal function is equivalent to {transfer}, and can be used to - * e.g. implement automatic token fees, slashing mechanisms, etc. + * @dev Moves `amount` of token `id` from `from` to `to` without checking for approvals. This function verifies + * that neither the sender nor the receiver are address(0), which means it cannot mint or burn tokens. + * Relies on the `_update` mechanism. * * Emits a {Transfer} event. * diff --git a/contracts/token/ERC6909/extensions/draft-ERC6909TokenSupply.sol b/contracts/token/ERC6909/extensions/draft-ERC6909TokenSupply.sol index 476935f8f..2ce0dd45b 100644 --- a/contracts/token/ERC6909/extensions/draft-ERC6909TokenSupply.sol +++ b/contracts/token/ERC6909/extensions/draft-ERC6909TokenSupply.sol @@ -26,7 +26,7 @@ contract ERC6909TokenSupply is ERC6909, IERC6909TokenSupply { } if (to == address(0)) { unchecked { - // amount <= _balances[id][from] <= _totalSupplies[id] + // amount <= _balances[from][id] <= _totalSupplies[id] _totalSupplies[id] -= amount; } } diff --git a/contracts/utils/Strings.sol b/contracts/utils/Strings.sol index 13d6e7b39..f8b0cc703 100644 --- a/contracts/utils/Strings.sol +++ b/contracts/utils/Strings.sol @@ -438,6 +438,10 @@ library Strings { * @dev Escape special characters in JSON strings. This can be useful to prevent JSON injection in NFT metadata. * * WARNING: This function should only be used in double quoted JSON strings. Single quotes are not escaped. + * + * NOTE: This function escapes all unicode characters, and not just the ones in ranges defined in section 2.5 of + * RFC-4627 (U+0000 to U+001F, U+0022 and U+005C). ECMAScript's `JSON.parse` does recover escaped unicode + * characters that are not in this range, but other tooling may provide different results. */ function escapeJSON(string memory input) internal pure returns (string memory) { bytes memory buffer = bytes(input); diff --git a/contracts/utils/structs/MerkleTree.sol b/contracts/utils/structs/MerkleTree.sol index c48a1247a..32e118b09 100644 --- a/contracts/utils/structs/MerkleTree.sol +++ b/contracts/utils/structs/MerkleTree.sol @@ -178,7 +178,7 @@ library MerkleTree { * root (before the update) and "new" root (after the update). The caller must verify that the reconstructed old * root is the last known one. * - * The `proof` must be an up-to-date inclusion proof for the leaf being update. This means that this function is + * The `proof` must be an up-to-date inclusion proof for the leaf being updated. This means that this function is * vulnerable to front-running. Any {push} or {update} operation (that changes the root of the tree) would render * all "in flight" updates invalid. * diff --git a/test/access/manager/AuthorityUtils.test.js b/test/access/manager/AuthorityUtils.test.js index 905913f14..465f617ad 100644 --- a/test/access/manager/AuthorityUtils.test.js +++ b/test/access/manager/AuthorityUtils.test.js @@ -2,6 +2,8 @@ const { ethers } = require('hardhat'); const { expect } = require('chai'); const { loadFixture } = require('@nomicfoundation/hardhat-network-helpers'); +const { MAX_UINT32, MAX_UINT64 } = require('../../helpers/constants'); + async function fixture() { const [user, other] = await ethers.getSigners(); @@ -70,7 +72,7 @@ describe('AuthorityUtils', function () { }); for (const immediate of [true, false]) { - for (const delay of [0n, 42n]) { + for (const delay of [0n, 42n, MAX_UINT32]) { it(`returns (immediate=${immediate}, delay=${delay})`, async function () { await this.authority._setImmediate(immediate); await this.authority._setDelay(delay); @@ -80,6 +82,14 @@ describe('AuthorityUtils', function () { }); } } + + it('out of bound delay', async function () { + await this.authority._setImmediate(false); + await this.authority._setDelay(MAX_UINT64); // bigger than the expected uint32 + const result = await this.mock.$canCallWithDelay(this.authority, this.user, this.other, '0x12345678'); + expect(result.immediate).to.equal(false); + expect(result.delay).to.equal(0n); + }); }); describe('when authority replies with empty data', function () { diff --git a/test/helpers/constants.js b/test/helpers/constants.js index 4dfda5eab..eb9b43e55 100644 --- a/test/helpers/constants.js +++ b/test/helpers/constants.js @@ -1,4 +1,5 @@ module.exports = { + MAX_UINT32: 2n ** 32n - 1n, MAX_UINT48: 2n ** 48n - 1n, MAX_UINT64: 2n ** 64n - 1n, };