From 8199a37120acff1eeeb6d881229be2185da4d0ba Mon Sep 17 00:00:00 2001 From: baku-ccron Date: Fri, 21 Aug 2026 06:42:38 +0000 Subject: [PATCH] test(LibConvert): pin aliasing, source lifetime and the write bounds The two reference implementation tests compare only the value of the result, which leaves three behaviours unobserved. `unsafeToBytes` hands back the buffer it was given and rewrites the shared length prefix from a count of words to a count of bytes. That aliasing is the whole reason the source is unsafe to use afterwards, yet an implementation returning a copy passed the suite. Pin the pointer and the rewritten prefix. `unsafeTo16BitBytes` allocates its own result and leaves the source intact, which is the opposite of `unsafeToBytes` and worth stating. The packing loop writes whole words at two byte offsets, so it stops short of the end of what it allocated. A write past that end lands beyond the length the value comparison reads and so cannot be seen by comparing results. Place sentinels immediately above the allocation instead, and cover the lengths whose packed data exactly fills whole words, where one byte past the end of the data falls outside the allocation rather than into its padding. Co-Authored-By: Claude Fable 5 --- test/LibConvert.t.sol | 109 ++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 109 insertions(+) diff --git a/test/LibConvert.t.sol b/test/LibConvert.t.sol index 1553fdb..fc86f30 100644 --- a/test/LibConvert.t.sol +++ b/test/LibConvert.t.sol @@ -24,4 +24,113 @@ contract LibConvertTest is Test { LibConvert.unsafeTo16BitBytes(us) ); } + + /// The conversion hands back the buffer it was given rather than a copy of + /// it, and rewrites the shared length prefix from a count of words to a + /// count of bytes. Both halves of that are what make the source unsafe to + /// use afterwards, so both are pinned here. + function testUnsafeToBytesAliasesTheSourceAndRewritesItsLengthPrefix(uint256[] memory us) public pure { + uint256 sourcePointer; + assembly ("memory-safe") { + sourcePointer := us + } + uint256 wordLength = us.length; + + bytes memory bs = LibConvert.unsafeToBytes(us); + + uint256 bytesPointer; + assembly ("memory-safe") { + bytesPointer := bs + } + + assertEq(bytesPointer, sourcePointer); + assertEq(bs.length, wordLength * 32); + + uint256 prefix; + assembly ("memory-safe") { + prefix := mload(sourcePointer) + } + assertEq(prefix, wordLength * 32); + } + + /// The 16 bit packing allocates its own result, so unlike `unsafeToBytes` it + /// reads the source and leaves it intact. + function testUnsafeTo16BitBytesLeavesTheSourceIntact(uint256[] memory us) public pure { + uint256[] memory expected = new uint256[](us.length); + for (uint256 i = 0; i < us.length; i++) { + expected[i] = us[i]; + } + + bytes memory bs = LibConvert.unsafeTo16BitBytes(us); + assertEq(bs.length, expected.length * 2); + + assertEq(us.length, expected.length); + for (uint256 i = 0; i < expected.length; i++) { + assertEq(us[i], expected[i]); + } + } + + /// The packing loop writes whole words at two byte offsets, so it has to + /// stop before it runs off the end of what it allocated. Comparing the + /// result value alone cannot see a write past that end, because such a write + /// lands beyond the length the comparison reads. Sentinels are placed + /// immediately above the allocation instead. + function testUnsafeTo16BitBytesWritesNothingPastItsAllocation(uint256[] memory us) public pure { + checkWritesNothingPastAllocation(us); + } + + /// Lengths whose packed data exactly fills whole words, where a single byte + /// written past the end of the data lands outside the allocation rather than + /// in its padding. + function testUnsafeTo16BitBytesWritesNothingPastItsAllocationAtWordBoundaries() public pure { + uint256[] memory lengths = new uint256[](4); + lengths[0] = 0; + lengths[1] = 16; + lengths[2] = 32; + lengths[3] = 48; + + for (uint256 j = 0; j < lengths.length; j++) { + uint256[] memory us = new uint256[](lengths[j]); + for (uint256 i = 0; i < us.length; i++) { + us[i] = type(uint256).max; + } + checkWritesNothingPastAllocation(us); + } + } + + function checkWritesNothingPastAllocation(uint256[] memory us) internal pure { + uint256 sentinel = uint256(keccak256("rain.lib.typecast.canary")); + uint256 expectedLength = us.length * 2; + + // Sentinels sit directly above the allocation the conversion is about to + // make, which is the free pointer plus a length word plus the data + // rounded up to whole words. + uint256 canary; + assembly { + canary := add(mload(0x40), add(0x20, and(add(mul(mload(us), 2), 0x1f), not(0x1f)))) + mstore(canary, sentinel) + mstore(add(canary, 0x20), sentinel) + mstore(add(canary, 0x40), sentinel) + mstore(add(canary, 0x60), sentinel) + } + + bytes memory bs = LibConvert.unsafeTo16BitBytes(us); + + uint256 c0; + uint256 c1; + uint256 c2; + uint256 c3; + assembly { + c0 := mload(canary) + c1 := mload(add(canary, 0x20)) + c2 := mload(add(canary, 0x40)) + c3 := mload(add(canary, 0x60)) + } + + assertEq(bs.length, expectedLength); + assertEq(c0, sentinel); + assertEq(c1, sentinel); + assertEq(c2, sentinel); + assertEq(c3, sentinel); + } }