Rename `arrayLengthPointer` to `arrayLengthOffset` and add changeset (#5371)

pull/5373/head
Ernesto García 2 months ago committed by GitHub
parent a71f79fbbc
commit 535b54da59
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
  1. 5
      .changeset/seven-insects-taste.md
  2. 16
      contracts/account/utils/draft-ERC7579Utils.sol

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

@ -182,14 +182,14 @@ library ERC7579Utils {
if (bufferLength < 32) revert ERC7579DecodingError();
// Get the offset of the array (pointer to the array length).
uint256 arrayLengthPointer = uint256(bytes32(executionCalldata[0:32]));
uint256 arrayLengthOffset = uint256(bytes32(executionCalldata[0:32]));
// The array length (at arrayLengthPointer) should be 32 bytes long. We check that this is within the
// 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 (arrayLengthPointer > bufferLength - 32) revert ERC7579DecodingError();
if (arrayLengthOffset > bufferLength - 32) revert ERC7579DecodingError();
// Get the array length. arrayLengthPointer + 32 is bounded by bufferLength so it does not overflow.
uint256 arrayLength = uint256(bytes32(executionCalldata[arrayLengthPointer:arrayLengthPointer + 32]));
// 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.
@ -197,13 +197,13 @@ library ERC7579Utils {
// - 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 arrayLengthPointer + 32, we can subtract with no overflow risk.
// 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 - arrayLengthPointer - 32 < arrayLength * 32)
if (arrayLength > type(uint64).max || bufferLength - arrayLengthOffset - 32 < arrayLength * 32)
revert ERC7579DecodingError();
assembly ("memory-safe") {
executionBatch.offset := add(add(executionCalldata.offset, arrayLengthPointer), 32)
executionBatch.offset := add(add(executionCalldata.offset, arrayLengthOffset), 32)
executionBatch.length := arrayLength
}
}

Loading…
Cancel
Save