Merge branch 'master' into assembly-consistency/add_0x20

.changeset/brave-islands-sparkle.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`GovernorSequentialProposalId`: Adds a `Governor` extension that sequentially numbers proposal ids instead of using the hash.

.changeset/cyan-taxis-travel.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`Address`: bubble up revert data on `sendValue` failed call

.changeset/seven-insects-taste.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': patch
+---
+
+`ERC7579Utils`: Add ABI decoding checks on calldata bounds within `decodeBatch`

.changeset/ten-fishes-fold.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`IGovernor`: Add the `getProposalId` function to the governor interface.

.changeset/thin-eels-cross.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': patch
+---
+
+`ERC4626`: Use the `asset` getter in `totalAssets`, `_deposit` and `_withdraw`.

.github/workflows/checks.yml

@@ -97,7 +97,7 @@ jobs:
         uses: ./.github/actions/setup
       - name: Run coverage
         run: npm run coverage
-      - uses: codecov/codecov-action@v4
+      - uses: codecov/codecov-action@v5
         env:
           CODECOV_TOKEN: ${{ secrets.CODECOV_TOKEN }}
 
@@ -118,11 +118,7 @@ jobs:
       - uses: actions/checkout@v4
       - name: Set up environment
         uses: ./.github/actions/setup
-      - run: rm foundry.toml
       - uses: crytic/slither-action@v0.4.0
-        with:
-          node-version: 18.15
-          slither-version: 0.10.1
 
   codespell:
     runs-on: ubuntu-latest

.github/workflows/formal-verification.yml

@@ -52,7 +52,7 @@ jobs:
       - name: Install python packages
         run: pip install -r fv-requirements.txt
       - name: Install java
-        uses: actions/setup-java@v3
+        uses: actions/setup-java@v4
         with:
           distribution: temurin
           java-version: ${{ env.JAVA_VERSION }}

.husky/pre-commit

@@ -0,0 +1,2 @@
+npm run test:generation
+npx lint-staged

audits/README.md

@@ -1,13 +1,14 @@
 # Audits
 
-| Date         | Version | Commit    | Auditor      | Scope                | Links                                                       |
-| ------------ | ------- | --------- | ------------ | -------------------- | ----------------------------------------------------------- |
-| October 2024 | v5.1.0  | TBD       | OpenZeppelin | v5.1 Changes         | [🔗](./2024-10-v5.1.pdf)                                    |
-| October 2023 | v5.0.0  | `b5a3e69` | OpenZeppelin | v5.0 Changes         | [🔗](./2023-10-v5.0.pdf)                                    |
-| May 2023     | v4.9.0  | `91df66c` | OpenZeppelin | v4.9 Changes         | [🔗](./2023-05-v4.9.pdf)                                    |
-| October 2022 | v4.8.0  | `14f98db` | OpenZeppelin | ERC4626, Checkpoints | [🔗](./2022-10-ERC4626.pdf) [🔗](./2022-10-Checkpoints.pdf) |
-| October 2018 | v2.0.0  | `dac5bcc` | LevelK       | Everything           | [🔗](./2018-10.pdf)                                         |
-| March 2017   | v1.0.4  | `9c5975a` | New Alchemy  | Everything           | [🔗](./2017-03.md)                                          |
+| Date          | Version | Commit                                                                           | Auditor      | Scope                | Links                                                       |
+| ------------- | ------- | -------------------------------------------------------------------------------- | ------------ | -------------------- | ----------------------------------------------------------- |
+| December 2024 | v5.2.0  | [`98d28f9`](https://github.com/openzeppelin/openzeppelin-contracts/tree/98d28f9) | OpenZeppelin | v5.2 Changes         | [🔗](./2024-12-v5.2.pdf)                                    |
+| October 2024  | v5.1.0  | [`aba9ff6`](https://github.com/openzeppelin/openzeppelin-contracts/tree/aba9ff6) | OpenZeppelin | v5.1 Changes         | [🔗](./2024-10-v5.1.pdf)                                    |
+| October 2023  | v5.0.0  | [`b5a3e69`](https://github.com/openzeppelin/openzeppelin-contracts/tree/b5a3e69) | OpenZeppelin | v5.0 Changes         | [🔗](./2023-10-v5.0.pdf)                                    |
+| May 2023      | v4.9.0  | [`91df66c`](https://github.com/openzeppelin/openzeppelin-contracts/tree/91df66c) | OpenZeppelin | v4.9 Changes         | [🔗](./2023-05-v4.9.pdf)                                    |
+| October 2022  | v4.8.0  | [`14f98db`](https://github.com/openzeppelin/openzeppelin-contracts/tree/14f98db) | OpenZeppelin | ERC4626, Checkpoints | [🔗](./2022-10-ERC4626.pdf) [🔗](./2022-10-Checkpoints.pdf) |
+| October 2018  | v2.0.0  | [`dac5bcc`](https://github.com/openzeppelin/openzeppelin-contracts/tree/dac5bcc) | LevelK       | Everything           | [🔗](./2018-10.pdf)                                         |
+| March 2017    | v1.0.4  | [`9c5975a`](https://github.com/openzeppelin/openzeppelin-contracts/tree/9c5975a) | New Alchemy  | Everything           | [🔗](./2017-03.md)                                          |
 
 # Formal Verification
 

contracts/account/utils/draft-ERC4337Utils.sol

@@ -24,9 +24,9 @@ library ERC4337Utils {
     function parseValidationData(
         uint256 validationData
     ) internal pure returns (address aggregator, uint48 validAfter, uint48 validUntil) {
-        validAfter = uint48(bytes32(validationData).extract_32_6(0x00));
-        validUntil = uint48(bytes32(validationData).extract_32_6(0x06));
-        aggregator = address(bytes32(validationData).extract_32_20(0x0c));
+        validAfter = uint48(bytes32(validationData).extract_32_6(0));
+        validUntil = uint48(bytes32(validationData).extract_32_6(6));
+        aggregator = address(bytes32(validationData).extract_32_20(12));
         if (validUntil == 0) validUntil = type(uint48).max;
     }
 
@@ -59,7 +59,8 @@ library ERC4337Utils {
         (address aggregator1, uint48 validAfter1, uint48 validUntil1) = parseValidationData(validationData1);
         (address aggregator2, uint48 validAfter2, uint48 validUntil2) = parseValidationData(validationData2);
 
-        bool success = aggregator1 == address(0) && aggregator2 == address(0);
+        bool success = aggregator1 == address(uint160(SIG_VALIDATION_SUCCESS)) &&
+            aggregator2 == address(uint160(SIG_VALIDATION_SUCCESS));
         uint48 validAfter = uint48(Math.max(validAfter1, validAfter2));
         uint48 validUntil = uint48(Math.min(validUntil1, validUntil2));
         return packValidationData(success, validAfter, validUntil);
@@ -110,22 +111,22 @@ library ERC4337Utils {
 
     /// @dev Returns `verificationGasLimit` from the {PackedUserOperation}.
     function verificationGasLimit(PackedUserOperation calldata self) internal pure returns (uint256) {
-        return uint128(self.accountGasLimits.extract_32_16(0x00));
+        return uint128(self.accountGasLimits.extract_32_16(0));
     }
 
     /// @dev Returns `accountGasLimits` from the {PackedUserOperation}.
     function callGasLimit(PackedUserOperation calldata self) internal pure returns (uint256) {
-        return uint128(self.accountGasLimits.extract_32_16(0x10));
+        return uint128(self.accountGasLimits.extract_32_16(16));
     }
 
     /// @dev Returns the first section of `gasFees` from the {PackedUserOperation}.
     function maxPriorityFeePerGas(PackedUserOperation calldata self) internal pure returns (uint256) {
-        return uint128(self.gasFees.extract_32_16(0x00));
+        return uint128(self.gasFees.extract_32_16(0));
     }
 
     /// @dev Returns the second section of `gasFees` from the {PackedUserOperation}.
     function maxFeePerGas(PackedUserOperation calldata self) internal pure returns (uint256) {
-        return uint128(self.gasFees.extract_32_16(0x10));
+        return uint128(self.gasFees.extract_32_16(16));
     }
 
     /// @dev Returns the total gas price for the {PackedUserOperation} (ie. `maxFeePerGas` or `maxPriorityFeePerGas + basefee`).
@@ -153,7 +154,7 @@ library ERC4337Utils {
         return self.paymasterAndData.length < 52 ? 0 : uint128(bytes16(self.paymasterAndData[36:52]));
     }
 
-    /// @dev Returns the forth section of `paymasterAndData` from the {PackedUserOperation}.
+    /// @dev Returns the fourth section of `paymasterAndData` from the {PackedUserOperation}.
     function paymasterData(PackedUserOperation calldata self) internal pure returns (bytes calldata) {
         return self.paymasterAndData.length < 52 ? _emptyCalldataBytes() : self.paymasterAndData[52:];
     }

contracts/account/utils/draft-ERC7579Utils.sol

@@ -38,9 +38,10 @@ library ERC7579Utils {
 
     /**
      * @dev Emits when an {EXECTYPE_TRY} execution fails.
-     * @param batchExecutionIndex The index of the failed transaction in the execution batch.
+     * @param batchExecutionIndex The index of the failed call in the execution batch.
+     * @param returndata The returned data from the failed call.
      */
-    event ERC7579TryExecuteFail(uint256 batchExecutionIndex, bytes result);
+    event ERC7579TryExecuteFail(uint256 batchExecutionIndex, bytes returndata);
 
     /// @dev The provided {CallType} is not supported.
     error ERC7579UnsupportedCallType(CallType callType);
@@ -60,10 +61,13 @@ library ERC7579Utils {
     /// @dev The module type is not supported.
     error ERC7579UnsupportedModuleType(uint256 moduleTypeId);
 
+    /// @dev Input calldata not properly formatted and possibly malicious.
+    error ERC7579DecodingError();
+
     /// @dev Executes a single call.
     function execSingle(
-        ExecType execType,
-        bytes calldata executionCalldata
+        bytes calldata executionCalldata,
+        ExecType execType
     ) internal returns (bytes[] memory returnData) {
         (address target, uint256 value, bytes calldata callData) = decodeSingle(executionCalldata);
         returnData = new bytes[](1);
@@ -72,8 +76,8 @@ library ERC7579Utils {
 
     /// @dev Executes a batch of calls.
     function execBatch(
-        ExecType execType,
-        bytes calldata executionCalldata
+        bytes calldata executionCalldata,
+        ExecType execType
     ) internal returns (bytes[] memory returnData) {
         Execution[] calldata executionBatch = decodeBatch(executionCalldata);
         returnData = new bytes[](executionBatch.length);
@@ -90,8 +94,8 @@ library ERC7579Utils {
 
     /// @dev Executes a delegate call.
     function execDelegateCall(
-        ExecType execType,
-        bytes calldata executionCalldata
+        bytes calldata executionCalldata,
+        ExecType execType
     ) internal returns (bytes[] memory returnData) {
         (address target, bytes calldata callData) = decodeDelegate(executionCalldata);
         returnData = new bytes[](1);
@@ -168,12 +172,40 @@ library ERC7579Utils {
     }
 
     /// @dev Decodes a batch of executions. See {encodeBatch}.
+    ///
+    /// NOTE: This function runs some checks and will throw a {ERC7579DecodingError} if the input is not properly formatted.
     function decodeBatch(bytes calldata executionCalldata) internal pure returns (Execution[] calldata executionBatch) {
-        assembly ("memory-safe") {
-            let ptr := add(executionCalldata.offset, calldataload(executionCalldata.offset))
-            // Extract the ERC7579 Executions
-            executionBatch.offset := add(ptr, 0x20)
-            executionBatch.length := calldataload(ptr)
+        unchecked {
+            uint256 bufferLength = executionCalldata.length;
+
+            // Check executionCalldata is not empty.
+            if (bufferLength < 32) revert ERC7579DecodingError();
+
+            // Get the offset of the array (pointer to the array length).
+            uint256 arrayLengthOffset = uint256(bytes32(executionCalldata[0:32]));
+
+            // The array length (at arrayLengthOffset) should be 32 bytes long. We check that this is within the
+            // buffer bounds. Since we know bufferLength is at least 32, we can subtract with no overflow risk.
+            if (arrayLengthOffset > bufferLength - 32) revert ERC7579DecodingError();
+
+            // Get the array length. arrayLengthOffset + 32 is bounded by bufferLength so it does not overflow.
+            uint256 arrayLength = uint256(bytes32(executionCalldata[arrayLengthOffset:arrayLengthOffset + 32]));
+
+            // Check that the buffer is long enough to store the array elements as "offset pointer":
+            // - each element of the array is an "offset pointer" to the data.
+            // - each "offset pointer" (to an array element) takes 32 bytes.
+            // - validity of the calldata at that location is checked when the array element is accessed, so we only
+            //   need to check that the buffer is large enough to hold the pointers.
+            //
+            // Since we know bufferLength is at least arrayLengthOffset + 32, we can subtract with no overflow risk.
+            // Solidity limits length of such arrays to 2**64-1, this guarantees `arrayLength * 32` does not overflow.
+            if (arrayLength > type(uint64).max || bufferLength - arrayLengthOffset - 32 < arrayLength * 32)
+                revert ERC7579DecodingError();
+
+            assembly ("memory-safe") {
+                executionBatch.offset := add(add(executionCalldata.offset, arrayLengthOffset), 0x20)
+                executionBatch.length := arrayLength
+            }
         }
     }
 

contracts/governance/Governor.sol

@@ -92,6 +92,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
     function supportsInterface(bytes4 interfaceId) public view virtual override(IERC165, ERC165) returns (bool) {
         return
             interfaceId == type(IGovernor).interfaceId ||
+            interfaceId == type(IGovernor).interfaceId ^ IGovernor.getProposalId.selector ||
             interfaceId == type(IERC1155Receiver).interfaceId ||
             super.supportsInterface(interfaceId);
     }
@@ -132,6 +133,18 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         return uint256(keccak256(abi.encode(targets, values, calldatas, descriptionHash)));
     }
 
+    /**
+     * @dev See {IGovernor-getProposalId}.
+     */
+    function getProposalId(
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        bytes32 descriptionHash
+    ) public view virtual returns (uint256) {
+        return hashProposal(targets, values, calldatas, descriptionHash);
+    }
+
     /**
      * @dev See {IGovernor-state}.
      */
@@ -317,7 +330,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         string memory description,
         address proposer
     ) internal virtual returns (uint256 proposalId) {
-        proposalId = hashProposal(targets, values, calldatas, keccak256(bytes(description)));
+        proposalId = getProposalId(targets, values, calldatas, keccak256(bytes(description)));
 
         if (targets.length != values.length || targets.length != calldatas.length || targets.length == 0) {
             revert GovernorInvalidProposalLength(targets.length, calldatas.length, values.length);
@@ -358,7 +371,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         bytes[] memory calldatas,
         bytes32 descriptionHash
     ) public virtual returns (uint256) {
-        uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
+        uint256 proposalId = getProposalId(targets, values, calldatas, descriptionHash);
 
         _validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Succeeded));
 
@@ -406,7 +419,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         bytes[] memory calldatas,
         bytes32 descriptionHash
     ) public payable virtual returns (uint256) {
-        uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
+        uint256 proposalId = getProposalId(targets, values, calldatas, descriptionHash);
 
         _validateStateBitmap(
             proposalId,
@@ -468,8 +481,8 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
     ) public virtual returns (uint256) {
         // The proposalId will be recomputed in the `_cancel` call further down. However we need the value before we
         // do the internal call, because we need to check the proposal state BEFORE the internal `_cancel` call
-        // changes it. The `hashProposal` duplication has a cost that is limited, and that we accept.
-        uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
+        // changes it. The `getProposalId` duplication has a cost that is limited, and that we accept.
+        uint256 proposalId = getProposalId(targets, values, calldatas, descriptionHash);
 
         // public cancel restrictions (on top of existing _cancel restrictions).
         _validateStateBitmap(proposalId, _encodeStateBitmap(ProposalState.Pending));
@@ -492,7 +505,7 @@ abstract contract Governor is Context, ERC165, EIP712, Nonces, IGovernor, IERC72
         bytes[] memory calldatas,
         bytes32 descriptionHash
     ) internal virtual returns (uint256) {
-        uint256 proposalId = hashProposal(targets, values, calldatas, descriptionHash);
+        uint256 proposalId = getProposalId(targets, values, calldatas, descriptionHash);
 
         _validateStateBitmap(
             proposalId,

contracts/governance/IGovernor.sol

@@ -203,7 +203,9 @@ interface IGovernor is IERC165, IERC6372 {
 
     /**
      * @notice module:core
-     * @dev Hashing function used to (re)build the proposal id from the proposal details..
+     * @dev Hashing function used to (re)build the proposal id from the proposal details.
+     *
+     * NOTE: For all off-chain and external calls, use {getProposalId}.
      */
     function hashProposal(
         address[] memory targets,
@@ -212,6 +214,17 @@ interface IGovernor is IERC165, IERC6372 {
         bytes32 descriptionHash
     ) external pure returns (uint256);
 
+    /**
+     * @notice module:core
+     * @dev Function used to get the proposal id from the proposal details.
+     */
+    function getProposalId(
+        address[] memory targets,
+        uint256[] memory values,
+        bytes[] memory calldatas,
+        bytes32 descriptionHash
+    ) external view returns (uint256);
+
     /**
      * @notice module:core
      * @dev Current state of a proposal, following Compound's convention