diff --git a/.changeset/small-brooms-prove.md b/.changeset/small-brooms-prove.md new file mode 100644 index 0000000000..7207ef0911 --- /dev/null +++ b/.changeset/small-brooms-prove.md @@ -0,0 +1,6 @@ +--- +"@latticexyz/store": patch +--- + +Fixed M-04 Memory Corruption on Load From Storage +It only affected external use of `Storage.load` with a `memoryPointer` argument diff --git a/packages/store/gas-report.json b/packages/store/gas-report.json index a1d1ad39b1..9f4e108ac7 100644 --- a/packages/store/gas-report.json +++ b/packages/store/gas-report.json @@ -285,7 +285,7 @@ "file": "test/GasStorageLoad.t.sol", "test": "testCompareStorageLoadMUD", "name": "MUD storage load (cold, 1 word, partial)", - "gasUsed": 2460 + "gasUsed": 2484 }, { "file": "test/GasStorageLoad.t.sol", @@ -309,7 +309,7 @@ "file": "test/GasStorageLoad.t.sol", "test": "testCompareStorageLoadMUD", "name": "MUD storage load (warm, 1 word, partial)", - "gasUsed": 460 + "gasUsed": 484 }, { "file": "test/GasStorageLoad.t.sol", @@ -609,13 +609,13 @@ "file": "test/Storage.t.sol", "test": "testStoreLoad", "name": "store 34 bytes over 3 storage slots (with offset and safeTrail))", - "gasUsed": 23009 + "gasUsed": 23013 }, { "file": "test/Storage.t.sol", "test": "testStoreLoad", "name": "load 34 bytes over 3 storage slots (with offset and safeTrail))", - "gasUsed": 865 + "gasUsed": 885 }, { "file": "test/StoreCoreDynamic.t.sol", @@ -627,19 +627,19 @@ "file": "test/StoreCoreDynamic.t.sol", "test": "testGetDynamicFieldSlice", "name": "get field slice (warm, 1 slot)", - "gasUsed": 1677 + "gasUsed": 1711 }, { "file": "test/StoreCoreDynamic.t.sol", "test": "testGetDynamicFieldSlice", "name": "get field slice (semi-cold, 1 slot)", - "gasUsed": 3680 + "gasUsed": 3704 }, { "file": "test/StoreCoreDynamic.t.sol", "test": "testGetDynamicFieldSlice", "name": "get field slice (warm, 2 slots)", - "gasUsed": 3907 + "gasUsed": 3931 }, { "file": "test/StoreCoreDynamic.t.sol", @@ -891,7 +891,7 @@ "file": "test/StoreCoreGas.t.sol", "test": "testSetAndGetField", "name": "get static field (overlap 2 slot)", - "gasUsed": 1865 + "gasUsed": 1889 }, { "file": "test/StoreCoreGas.t.sol", diff --git a/packages/store/src/Storage.sol b/packages/store/src/Storage.sol index 80950c3ee9..f74f98a72b 100644 --- a/packages/store/src/Storage.sol +++ b/packages/store/src/Storage.sol @@ -199,7 +199,12 @@ library Storage { wordRemainder = 32 - offset; } - uint256 mask = leftMask(wordRemainder); + uint256 mask; + if (length < wordRemainder) { + mask = leftMask(length); + } else { + mask = leftMask(wordRemainder); + } /// @solidity memory-safe-assembly assembly { // Load data from storage and offset it to match memory diff --git a/packages/store/test/Storage.t.sol b/packages/store/test/Storage.t.sol index e490a6cdb3..6d8e178a4e 100644 --- a/packages/store/test/Storage.t.sol +++ b/packages/store/test/Storage.t.sol @@ -93,4 +93,41 @@ contract StorageTest is Test, GasReporter { Storage.store({ storagePointer: storagePointer, offset: offset, data: abi.encodePacked((data)) }); assertEq(bytes16(Storage.loadField({ storagePointer: storagePointer, length: 16, offset: offset })), data); } + + function testStoreLoadToPointer() public { + uint256 memoryCorruptionCheck = 0x0101010101010101010101010101010101010101010101010101010101010101; + uint80 prefix = 0x01010101010101010101; + uint80 dataBeforeLoad = 0x00000000000000000000; + uint80 dataAfterLoad = 0xbeeeeeeeeeeeeeeeeeef; + uint256 postfix = 0x010101010101010101010101; + + bytes memory testData = abi.encodePacked( + memoryCorruptionCheck, + prefix, + dataBeforeLoad, + postfix, + memoryCorruptionCheck + ); + bytes memory expectedData = abi.encodePacked( + memoryCorruptionCheck, + prefix, + dataAfterLoad, + postfix, + memoryCorruptionCheck + ); + uint256 memoryPointer; + /// @solidity memory-safe-assembly + assembly { + memoryPointer := add(testData, 0x20) + } + // skip prefixes + memoryPointer += 32 + 10; + + uint256 storagePointer = uint256(keccak256("some location")); + Storage.store({ storagePointer: storagePointer, offset: 10, data: abi.encodePacked(dataAfterLoad) }); + + Storage.load({ storagePointer: storagePointer, length: 10, offset: 10, memoryPointer: memoryPointer }); + + assertEq(testData, expectedData); + } }