From 6065eb04f298945c8f4216852d7b37857793a088 Mon Sep 17 00:00:00 2001
From: highskore <luka@racki.dev>
Date: Thu, 19 Dec 2024 17:01:22 +0100
Subject: [PATCH] fix(ExecutionLib): fix decodeBatch

---
 src/accounts/erc7579/lib/ExecutionLib.sol | 60 +++++++++++++++++------
 1 file changed, 45 insertions(+), 15 deletions(-)

diff --git a/src/accounts/erc7579/lib/ExecutionLib.sol b/src/accounts/erc7579/lib/ExecutionLib.sol
index 49b345cd..1685c8ed 100644
--- a/src/accounts/erc7579/lib/ExecutionLib.sol
+++ b/src/accounts/erc7579/lib/ExecutionLib.sol
@@ -9,25 +9,55 @@ import { Execution } from "../../common/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
+            }
         }
     }