Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 11 additions & 1 deletion src/utils/LibBytes.sol
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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) }
}
}

Expand Down
88 changes: 88 additions & 0 deletions test/LibBytesEdgeCases.t.sol
Original file line number Diff line number Diff line change
@@ -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;
}
}