From cb22794c9fd56e669e3ef1f608ad7dc890c23b42 Mon Sep 17 00:00:00 2001
From: highskore <luka@racki.dev>
Date: Thu, 19 Dec 2024 16:55:28 +0100
Subject: [PATCH] fix(ExecutionLib): fix decodeBatch
---
src/lib/ExecutionLib.sol | 60 ++++++++++++++++++++++++++++++----------
1 file changed, 45 insertions(+), 15 deletions(-)
diff --git a/src/lib/ExecutionLib.sol b/src/lib/ExecutionLib.sol
index 3d509eb..f41366e 100644
--- a/src/lib/ExecutionLib.sol
+++ b/src/lib/ExecutionLib.sol
@@ -8,25 +8,55 @@ import { Execution } from "../interfaces/IERC7579Account.sol";
* malloc for memory allocation is bad for gas. use this assembly instead
*/
library ExecutionLib {
- function decodeBatch(bytes calldata callData)
+ error ERC7579DecodingError();
+
+ function decodeBatch(bytes calldata executionCalldata)
internal
pure
returns (Execution[] calldata executionBatch)
{
- /*
- * Batch Call Calldata Layout
- * Offset (in bytes) | Length (in bytes) | Contents
- * 0x0 | 0x4 | bytes4 function selector
- * 0x4 | - |
- abi.encode(IERC7579Execution.Execution[])
- */
- // solhint-disable-next-line no-inline-assembly
- assembly ("memory-safe") {
- let dataPointer := add(callData.offset, calldataload(callData.offset))
-
- // Extract the ERC7579 Executions
- executionBatch.offset := add(dataPointer, 32)
- executionBatch.length := calldataload(dataPointer)
+ 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 arrayLengthPointer = uint256(bytes32(executionCalldata[0:32]));
+
+ // The array length (at arrayLengthPointer) 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();
+
+ // Get the array length. arrayLengthPointer + 32 is bounded by bufferLength so it does
+ // not overflow.
+ uint256 arrayLength =
+ uint256(bytes32(executionCalldata[arrayLengthPointer:arrayLengthPointer + 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 arrayLengthPointer + 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
+ ) {
+ revert ERC7579DecodingError();
+ }
+
+ assembly ("memory-safe") {
+ executionBatch.offset := add(add(executionCalldata.offset, arrayLengthPointer), 32)
+ executionBatch.length := arrayLength
+ }
}
}