From 11faffa1b5f897bb33e3edf64e5d156b8928ad0a Mon Sep 17 00:00:00 2001 From: dk1a Date: Thu, 30 Nov 2023 12:44:55 +0300 Subject: [PATCH 1/5] fix(store): M-04 --- packages/store/gas-report.json | 16 ++++++++-------- packages/store/src/Storage.sol | 7 ++++++- 2 files changed, 14 insertions(+), 9 deletions(-) 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 From 2dd984ee6b2a2988ff341311f1b0c2421bd8f341 Mon Sep 17 00:00:00 2001 From: dk1a Date: Thu, 30 Nov 2023 14:05:17 +0300 Subject: [PATCH 2/5] Create small-brooms-prove.md --- .changeset/small-brooms-prove.md | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 .changeset/small-brooms-prove.md 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 From 82cc497d1151c355b208430d0b5a5ee1783c3414 Mon Sep 17 00:00:00 2001 From: dk1a Date: Thu, 30 Nov 2023 14:27:31 +0300 Subject: [PATCH 3/5] gas-report --- packages/store/gas-report.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/store/gas-report.json b/packages/store/gas-report.json index 9f4e108ac7..37a916b0d1 100644 --- a/packages/store/gas-report.json +++ b/packages/store/gas-report.json @@ -609,13 +609,13 @@ "file": "test/Storage.t.sol", "test": "testStoreLoad", "name": "store 34 bytes over 3 storage slots (with offset and safeTrail))", - "gasUsed": 23013 + "gasUsed": 23009 }, { "file": "test/Storage.t.sol", "test": "testStoreLoad", "name": "load 34 bytes over 3 storage slots (with offset and safeTrail))", - "gasUsed": 885 + "gasUsed": 889 }, { "file": "test/StoreCoreDynamic.t.sol", From 5dd70ad49f7453157b918f8915b98d08b1086e16 Mon Sep 17 00:00:00 2001 From: dk1a Date: Thu, 30 Nov 2023 14:31:15 +0300 Subject: [PATCH 4/5] test --- packages/store/test/Storage.t.sol | 37 +++++++++++++++++++++++++++++++ 1 file changed, 37 insertions(+) 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); + } } From 63599ff3985c59a0dfef7cec6229e41e35ef0740 Mon Sep 17 00:00:00 2001 From: dk1a Date: Thu, 30 Nov 2023 14:38:39 +0300 Subject: [PATCH 5/5] gas-report --- packages/store/gas-report.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/store/gas-report.json b/packages/store/gas-report.json index 37a916b0d1..9f4e108ac7 100644 --- a/packages/store/gas-report.json +++ b/packages/store/gas-report.json @@ -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": 889 + "gasUsed": 885 }, { "file": "test/StoreCoreDynamic.t.sol",