From 14bc59c7f42d3eba421191c26c4a136b537380d3 Mon Sep 17 00:00:00 2001 From: Letta Integration <300689746+letta-integration[bot]@users.noreply.github.com> Date: Wed, 9 Sep 2026 21:40:47 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=90=9E=20Fix=20LibBytes.split=20null=20pa?= =?UTF-8?q?th=20and=20dynamicStructInCalldata=20bounds=20check?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - indicesOf: when needle is longer than subject, allocate a proper empty array instead of returning a null pointer. Previously, split read the array length from the scratch space at 0x00, which is not guaranteed to be zero, causing memory corruption or OOG on the null path (e.g. split("", ",")). - dynamicStructInCalldata: add the missing gt(s, l) bounds check, mirroring bytesInCalldata. Previously a struct offset s > a.length was not rejected and result.length = a.length - s wrapped around. Co-Authored-By: Letta Code --- src/utils/LibBytes.sol | 12 ++++- test/LibBytesEdgeCases.t.sol | 88 ++++++++++++++++++++++++++++++++++++ 2 files changed, 99 insertions(+), 1 deletion(-) create mode 100644 test/LibBytesEdgeCases.t.sol diff --git a/src/utils/LibBytes.sol b/src/utils/LibBytes.sol index f413b68b8..1dd8e75e6 100644 --- a/src/utils/LibBytes.sol +++ b/src/utils/LibBytes.sol @@ -546,6 +546,15 @@ library LibBytes { /// @solidity memory-safe-assembly assembly { let searchLen := mload(needle) + if gt(searchLen, mload(subject)) { + // Allocate a proper empty array instead of returning a null pointer, + // so that downstream consumers (e.g. `split`) never read the + // scratch space at 0x00 to determine the array length. + // We allocate one more word, so this array can be recycled for {split}. + result := mload(0x40) + mstore(result, 0) + mstore(0x40, add(result, 0x40)) + } if iszero(gt(searchLen, mload(subject))) { result := mload(0x40) let i := add(subject, 0x20) @@ -814,7 +823,8 @@ library LibBytes { let s := calldataload(add(a.offset, offset)) // Relative offset of `result` from `a.offset`. result.offset := add(a.offset, s) result.length := sub(a.length, s) - if or(shr(64, or(s, or(l, a.offset))), gt(offset, l)) { revert(l, 0x00) } + // forgefmt: disable-next-item + if or(shr(64, or(s, or(l, a.offset))), or(gt(offset, l), gt(s, l))) { revert(l, 0x00) } } } diff --git a/test/LibBytesEdgeCases.t.sol b/test/LibBytesEdgeCases.t.sol new file mode 100644 index 000000000..428480453 --- /dev/null +++ b/test/LibBytesEdgeCases.t.sol @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: MIT + +pragma solidity ^0.8.4; + +import "./utils/forge-std/Test.sol"; +import {LibBytes} from "../src/utils/LibBytes.sol"; +import {EfficientHashLib} from "../src/utils/EfficientHashLib.sol"; + +/// @dev Tests for LibBytes edge cases: +/// - `split` when the delimiter is longer than the subject, +/// - `dynamicStructInCalldata` with a malformed struct offset. +contract LibBytesEdgeCasesTest is Test { + /// ------------------------------------------------------------------ + /// `split` with delimiter longer than subject. + /// Previously, `indicesOf` returned a null pointer on this path, and + /// `split` read the array length from the scratch space at 0x00, + /// which is not guaranteed to be zero. + /// ------------------------------------------------------------------ + /// Clean-scratch sanity: the null path works when scratch is zero. + function testSplitNullPathCleanScratch() public pure { + bytes[] memory p = LibBytes.split("ab", "xyz"); + require(p.length == 1, "length"); + require(bytes(p[0]).length == 2, "element length"); + } + + /// Scratch dirtied by a prior call to another library function. + /// Before the fix: corrupted result or OOG. After the fix: correct result. + function testSplitNullPathAfterHash() public { + bytes32 h = EfficientHashLib.hash(uint256(1)); + h; // silence unused + bytes[] memory p = LibBytes.split("ab", "xyz"); + assertEq(p.length, 1, "array length must be 1"); + assertEq(bytes(p[0]), "ab", "element must equal subject"); + } + + /// Two consecutive null-path splits: the first split itself leaves + /// scratch dirty for the second. Before the fix: OOG revert. + function testSplitNullPathTwoConsecutive() public pure { + bytes[] memory a = LibBytes.split("ab", "xyz"); + require(a.length == 1, "first split length"); + require(bytes(a[0]).length == 2, "first split element"); + bytes[] memory b = LibBytes.split("cd", "wxyz"); + require(b.length == 1, "second split length"); + require(bytes(b[0]).length == 2, "second split element"); + } + + /// ------------------------------------------------------------------ + /// `dynamicStructInCalldata` with a malformed struct offset. + /// Previously, a struct offset `s > a.length` was not rejected, and + /// `result.length = a.length - s` wrapped around to a huge value. + /// ------------------------------------------------------------------ + + Harness harness; + + function setUp() public { + harness = new Harness(); + } + + /// Malformed struct offset: a = 64 bytes, first word = 65 (> a.length). + /// Before the fix: no revert from LibBytes, returned slice had + /// length 2**256 - 1. After the fix: reverts with an empty reason + /// (the assembly `revert(l, 0x00)` bounds check). + function testDynamicStructInCalldataMalformedOffset() public { + bytes memory payload = abi.encode(uint256(65), uint256(0)); // 64 bytes, s = 65 + (bool ok, bytes memory reason) = + address(harness).call(abi.encodeWithSelector(Harness.f.selector, payload)); + assertFalse(ok, "must revert on out-of-bounds struct offset"); + assertEq(reason, bytes(""), "revert must come from LibBytes bounds check"); + } + + /// Valid input sanity: must NOT revert. + function testDynamicStructInCalldataValid() public view { + bytes memory payload = abi.encode(uint256(0x20), uint256(123)); + harness.f(payload); + } +} + +contract Harness { + /// Re-encodes the returned slice so the wrapped length is actually consumed. + function f(bytes calldata a) external pure returns (bytes calldata) { + bytes calldata s = LibBytes.dynamicStructInCalldata(a, 0); + // Touch the slice: reading its length forces use of the wrapped value. + require( + s.length < 0xffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffffff, "wrapped" + ); + return s; + } +}