From fa8785f917d59f1dc60cb98b2188e44fb80587fe Mon Sep 17 00:00:00 2001 From: yonkoo11 Date: Sat, 11 Jul 2026 21:19:52 +0100 Subject: [PATCH] fix(StdStorage): find string/bytes slots (dynamic ABI returns) find() compared only a single returndata word at depth. For string/bytes getters that word is the ABI offset (0x20), so mutating storage never looked like a hit and short strings failed with "Slot(s) not found". For single dynamic returns, compare the full returndata hash when probing slots, and skip the raw slot==return equality check (storage encoding is not the ABI word). Static multi-word returns still use the depth word so struct field isolation is unchanged. Fixes #345 --- src/StdStorage.sol | 60 +++++++++++++++++++++++++++++++++++++------ test/StdStorage.t.sol | 34 ++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 8 deletions(-) diff --git a/src/StdStorage.sol b/src/StdStorage.sol index 3035880e..ed940b10 100644 --- a/src/StdStorage.sol +++ b/src/StdStorage.sol @@ -51,20 +51,62 @@ library stdStorageSafe { return (success, result); } + /// @notice Staticcall the configured target and return raw returndata. + function callTargetRaw(StdStorage storage self) internal view returns (bool success, bytes memory rdat) { + bytes memory cd = abi.encodePacked(self._sig, getCallParams(self)); + (success, rdat) = self._target.staticcall(cd); + } + + /// @notice True when returndata is a single ABI dynamic value (string/bytes): word0 == 0x20 offset. + function _isSingleDynamicReturn(bytes memory rdat) private pure returns (bool) { + if (rdat.length < 64) { + return false; + } + return uint256(_bytesToBytes32(rdat, 0)) == 32; + } + /// @notice Returns whether mutating `slot` changes the return value of the configured target call. - /// @dev Temporarily writes either `type(uint256).max` or `0` to the slot to detect sensitivity. + /// @dev Temporarily writes 0 / type(uint256).max to the slot to detect sensitivity. + /// Static multi-word returns (structs) still compare only the word at `depth` so mutating a + /// sibling field is not a hit for this field. + /// Single dynamic returns (string/bytes) compare the full returndata hash: word0 is usually + /// just the ABI offset 0x20 and does not change when content changes (fixes #345). function checkSlotMutatesCall(StdStorage storage self, bytes32 slot) internal returns (bool) { bytes32 prevSlotValue = vm.load(self._target, slot); - (bool success, bytes32 prevReturnValue) = callTarget(self); + (bool prevSuccess, bytes memory prevRet) = callTargetRaw(self); + + if (prevSuccess && _isSingleDynamicReturn(prevRet)) { + vm.store(self._target, slot, bytes32(UINT256_MAX)); + (bool newSuccess, bytes memory newRet) = callTargetRaw(self); + bool changed = + (prevSuccess != newSuccess) || (prevSuccess && keccak256(prevRet) != keccak256(newRet)); + if (!changed) { + vm.store(self._target, slot, bytes32(0)); + (newSuccess, newRet) = callTargetRaw(self); + changed = + (prevSuccess != newSuccess) || (prevSuccess && keccak256(prevRet) != keccak256(newRet)); + } + vm.store(self._target, slot, prevSlotValue); + return changed; + } + bytes32 prevReturnValue = _bytesToBytes32(prevRet, 32 * self._depth); bytes32 testVal = prevReturnValue == bytes32(0) ? bytes32(UINT256_MAX) : bytes32(0); vm.store(self._target, slot, testVal); - - (, bytes32 newReturnValue) = callTarget(self); - + (, bytes memory newRetStatic) = callTargetRaw(self); + bytes32 newReturnValue = _bytesToBytes32(newRetStatic, 32 * self._depth); vm.store(self._target, slot, prevSlotValue); + return (prevSuccess && (prevReturnValue != newReturnValue)); + } - return (success && (prevReturnValue != newReturnValue)); + /// @dev Skip raw slot==return equality when the getter is a single dynamic ABI return + /// (string/bytes). `callResult` at depth 0 is the offset word (0x20) in that case. + function _skipSlotValueCheck(StdStorage storage self, bytes32 callResult) private view returns (bool) { + if (uint256(callResult) != 32) { + return false; + } + (bool ok, bytes memory rdat) = callTargetRaw(self); + return ok && _isSingleDynamicReturn(rdat); } /// @notice Searches for the bit offset of the packed variable within `slot` from the left or right side. @@ -146,10 +188,12 @@ library stdStorageSafe { } } - // Check that value between found offsets is equal to the current call result + // Check that value between found offsets is equal to the current call result. + // For string/bytes getters the word at depth 0 is the ABI offset (0x20), not the + // storage encoding — skip the raw equality check in that case (#345). uint256 curVal = (uint256(prev) & getMaskByOffsets(offsetLeft, offsetRight)) >> offsetRight; - if (uint256(callResult) != curVal) { + if (uint256(callResult) != curVal && !_skipSlotValueCheck(self, callResult)) { continue; } diff --git a/test/StdStorage.t.sol b/test/StdStorage.t.sol index ab87da38..869bf573 100644 --- a/test/StdStorage.t.sol +++ b/test/StdStorage.t.sol @@ -351,6 +351,25 @@ contract StdStorageTest is Test { assertEq(test.edgeCaseArray(0), 1); } + // Regression tests for https://github.com/foundry-rs/forge-std/issues/345 + function test_StorageFindShortString() public { + ShortStringStorage t = new ShortStringStorage(); + uint256 slot = stdstore.target(address(t)).sig("exists()").find(); + assertEq(slot, 0); + } + + function test_StorageFindLongString() public { + LongStringStorage t = new LongStringStorage(); + // Long strings SLOAD base + content slots; find must not revert (content slot is OK). + stdstore.target(address(t)).sig("exists()").find(); + } + + function test_StorageFindBytes() public { + BytesStorage t = new BytesStorage(); + uint256 slot = stdstore.target(address(t)).sig("data()").find(); + assertEq(slot, 0); + } + // Regression test for https://github.com/foundry-rs/forge-std/issues/740 // `find()` used to infinite-loop on tokens whose `balanceOf` reads multiple // storage slots and returns a derived value (reflection tokens). @@ -377,6 +396,21 @@ contract StorageTestTarget { } } + +contract ShortStringStorage { + // 31 chars => short string encoding (length in low byte) + string public exists = "thequickbrownfoxjumpsoverthelaz"; +} + +contract LongStringStorage { + string public exists = + "the quick brown fox jumps over the lazy dog and then keeps running so this is long"; +} + +contract BytesStorage { + bytes public data = hex"00112233445566778899aabbccddeeff00112233"; +} + contract ReflectionTokenTarget { using stdStorage for StdStorage;