Merge branch 'OpenZeppelin:master' into master

Author: Dargon789

Diff for: .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.

Diff for: .changeset/cyan-taxis-travel.md

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

Diff for: .changeset/long-walls-draw.md

@@ -0,0 +1,5 @@
+---
+'openzeppelin-solidity': minor
+---
+
+`IERC6909`: Add the interface for ERC-6909.

Diff for: .changeset/proud-planes-arrive.md

@@ -2,4 +2,4 @@
 'openzeppelin-solidity': minor
 ---
 
-`Bytes`: Add a library of common operation that operate on `bytes` objects.
+`Bytes`: Add a library of common operations that operate on `bytes` objects.

Diff for: .changeset/seven-insects-taste.md

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

Diff for: .changeset/ten-fishes-fold.md

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

Diff for: .changeset/thin-eels-cross.md

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

Diff for: .githooks/pre-push

@@ -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/[email protected]
-        with:
-          node-version: 18.15
-          slither-version: 0.10.1
 
   codespell:
     runs-on: ubuntu-latest

Diff for: .github/workflows/checks.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 }}

Diff for: .github/workflows/formal-verification.yml

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

Diff for: .husky/pre-commit

@@ -55,7 +55,7 @@ External contributions must be reviewed separately by multiple maintainers.
 
 Automation should be used as much as possible to reduce the possibility of human error and forgetfulness.
 
-Automations that make use of sensitive credentials must use secure secret management, and must be strengthened against attacks such as [those on GitHub Actions worklows](https://github.com/nikitastupin/pwnhub).
+Automations that make use of sensitive credentials must use secure secret management, and must be strengthened against attacks such as [those on GitHub Actions workflows](https://github.com/nikitastupin/pwnhub).
 
 Some other examples of automation are:
 

Diff for: GUIDELINES.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
 

Diff for: audits/2024-12-v5.2.pdf

@@ -2,7 +2,7 @@
 
 pragma solidity ^0.8.20;
 
-import {IEntryPoint, PackedUserOperation} from "../../interfaces/draft-IERC4337.sol";
+import {PackedUserOperation} from "../../interfaces/draft-IERC4337.sol";
 import {Math} from "../../utils/math/Math.sol";
 import {Packing} from "../../utils/Packing.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);
@@ -71,12 +72,7 @@ library ERC4337Utils {
         return (aggregator_, block.timestamp < validAfter || validUntil < block.timestamp);
     }
 
-    /// @dev Computes the hash of a user operation with the current entrypoint and chainid.
-    function hash(PackedUserOperation calldata self) internal view returns (bytes32) {
-        return hash(self, address(this), block.chainid);
-    }
-
-    /// @dev Sames as {hash}, but with a custom entrypoint and chainid.
+    /// @dev Computes the hash of a user operation for a given entrypoint and chainid.
     function hash(
         PackedUserOperation calldata self,
         address entrypoint,
@@ -103,24 +99,34 @@ library ERC4337Utils {
         return result;
     }
 
+    /// @dev Returns `factory` from the {PackedUserOperation}, or address(0) if the initCode is empty or not properly formatted.
+    function factory(PackedUserOperation calldata self) internal pure returns (address) {
+        return self.initCode.length < 20 ? address(0) : address(bytes20(self.initCode[0:20]));
+    }
+
+    /// @dev Returns `factoryData` from the {PackedUserOperation}, or empty bytes if the initCode is empty or not properly formatted.
+    function factoryData(PackedUserOperation calldata self) internal pure returns (bytes calldata) {
+        return self.initCode.length < 20 ? _emptyCalldataBytes() : self.initCode[20:];
+    }
+
     /// @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}.
+    /// @dev Returns `callGasLimit` 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`).
@@ -129,22 +135,35 @@ library ERC4337Utils {
             // Following values are "per gas"
             uint256 maxPriorityFee = maxPriorityFeePerGas(self);
             uint256 maxFee = maxFeePerGas(self);
-            return Math.ternary(maxFee == maxPriorityFee, maxFee, Math.min(maxFee, maxPriorityFee + block.basefee));
+            return Math.min(maxFee, maxPriorityFee + block.basefee);
         }
     }
 
     /// @dev Returns the first section of `paymasterAndData` from the {PackedUserOperation}.
     function paymaster(PackedUserOperation calldata self) internal pure returns (address) {
-        return address(bytes20(self.paymasterAndData[0:20]));
+        return self.paymasterAndData.length < 52 ? address(0) : address(bytes20(self.paymasterAndData[0:20]));
     }
 
     /// @dev Returns the second section of `paymasterAndData` from the {PackedUserOperation}.
     function paymasterVerificationGasLimit(PackedUserOperation calldata self) internal pure returns (uint256) {
-        return uint128(bytes16(self.paymasterAndData[20:36]));
+        return self.paymasterAndData.length < 52 ? 0 : uint128(bytes16(self.paymasterAndData[20:36]));
     }
 
     /// @dev Returns the third section of `paymasterAndData` from the {PackedUserOperation}.
     function paymasterPostOpGasLimit(PackedUserOperation calldata self) internal pure returns (uint256) {
-        return uint128(bytes16(self.paymasterAndData[36:52]));
+        return self.paymasterAndData.length < 52 ? 0 : uint128(bytes16(self.paymasterAndData[36:52]));
+    }
+
+    /// @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:];
+    }
+
+    // slither-disable-next-line write-after-write
+    function _emptyCalldataBytes() private pure returns (bytes calldata result) {
+        assembly ("memory-safe") {
+            result.offset := 0
+            result.length := 0
+        }
     }
 }

Diff for: audits/README.md

@@ -22,22 +22,26 @@ library ERC7579Utils {
     using Packing for *;
 
     /// @dev A single `call` execution.
-    CallType constant CALLTYPE_SINGLE = CallType.wrap(0x00);
+    CallType internal constant CALLTYPE_SINGLE = CallType.wrap(0x00);
 
     /// @dev A batch of `call` executions.
-    CallType constant CALLTYPE_BATCH = CallType.wrap(0x01);
+    CallType internal constant CALLTYPE_BATCH = CallType.wrap(0x01);
 
     /// @dev A `delegatecall` execution.
-    CallType constant CALLTYPE_DELEGATECALL = CallType.wrap(0xFF);
+    CallType internal constant CALLTYPE_DELEGATECALL = CallType.wrap(0xFF);
 
     /// @dev Default execution type that reverts on failure.
-    ExecType constant EXECTYPE_DEFAULT = ExecType.wrap(0x00);
+    ExecType internal constant EXECTYPE_DEFAULT = ExecType.wrap(0x00);
 
     /// @dev Execution type that does not revert on failure.
-    ExecType constant EXECTYPE_TRY = ExecType.wrap(0x01);
+    ExecType internal constant EXECTYPE_TRY = ExecType.wrap(0x01);
 
-    /// @dev Emits when an {EXECTYPE_TRY} execution fails.
-    event ERC7579TryExecuteFail(uint256 batchExecutionIndex, bytes result);
+    /**
+     * @dev Emits when an {EXECTYPE_TRY} execution fails.
+     * @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 returndata);
 
     /// @dev The provided {CallType} is not supported.
     error ERC7579UnsupportedCallType(CallType callType);
@@ -57,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);
@@ -69,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);
@@ -87,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);
@@ -165,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, 32)
-            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), 32)
+                executionBatch.length := arrayLength
+            }
         }
     }
 

Diff for: contracts/account/utils/draft-ERC4337Utils.sol

@@ -26,6 +26,11 @@ import {Ownable} from "../access/Ownable.sol";
  *
  * NOTE: When using this contract with any token whose balance is adjusted automatically (i.e. a rebase token), make
  * sure to account the supply/balance adjustment in the vesting schedule to ensure the vested amount is as intended.
+ *
+ * NOTE: Chains with support for native ERC20s may allow the vesting wallet to withdraw the underlying asset as both an
+ * ERC20 and as native currency. For example, if chain C supports token A and the wallet gets deposited 100 A, then
+ * at 50% of the vesting period, the beneficiary can withdraw 50 A as ERC20 and 25 A as native currency (totaling 75 A).
+ * Consider disabling one of the withdrawal methods.
  */
 contract VestingWallet is Context, Ownable {
     event EtherReleased(uint256 amount);