diff --git a/src/utils/LibBytes.sol b/src/utils/LibBytes.sol index 06d082b23..c9ea07c38 100644 --- a/src/utils/LibBytes.sol +++ b/src/utils/LibBytes.sol @@ -816,7 +816,11 @@ 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) } + // `gt(s, a.length)` is required: without it `sub(a.length, s)` underflows and + // `result.length` becomes ~2**256, yielding a slice that points past `a`. + // The sibling helpers already bound `s` -- `bytesInCalldata` via + // `gt(add(s, result.length), l)`, `staticStructInCalldata` via `gt(offset, l)`. + if or(shr(64, or(s, or(l, a.offset))), or(gt(offset, l), gt(s, a.length))) { revert(l, 0x00) } } } diff --git a/test/LibBytes.t.sol b/test/LibBytes.t.sol index ca94902f3..c7b3b4474 100644 --- a/test/LibBytes.t.sol +++ b/test/LibBytes.t.sol @@ -414,4 +414,25 @@ contract LibBytesTest is SoladyTest { require(keccak256(expectedChildren[i]) == keccak256(children[i])); } } + + function testDynamicStructInCalldataRejectsOutOfBoundsOffset() public { + // `s` (the relative offset read from calldata) greater than `a.length` used to make + // `sub(a.length, s)` underflow, returning a slice with a ~2**256 length pointing past `a`. + bytes memory encoded = abi.encodePacked(uint256(0x1000)); + vm.expectRevert(); + this.dynamicStructInCalldataAt(encoded, 0x00); + } + + function dynamicStructInCalldataAt(bytes calldata a, uint256 offset) + public + pure + returns (uint256 o, uint256 l) + { + bytes calldata p = LibBytes.dynamicStructInCalldata(a, offset); + assembly { + o := p.offset + l := p.length + } + } + }