fix(memory_instance): zero out heap memory when reallocating after reset (#941)

* fix(memory_instance): zero out heap memory when reallocating

* fix: pin base64ct

* fix: tests and saturating_sub

* fix: more deps

* fix: test

* fix: naming, fmt

Author: rymnc

.github/workflows/ci.yml

@@ -221,7 +221,7 @@ jobs:
           targets: "wasm32-unknown-unknown"
 
       - name: Installing required crates
-        run: cargo install wasm-bindgen-cli wasm-opt
+        run: cargo install wasm-bindgen-cli wasm-opt --locked
 
       - name: Setup PNPM
         uses: pnpm/action-setup@v4

CHANGELOG.md

@@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
 
 ## [Unreleased]
 
+### Fixed
+
+- [941](https://github.com/FuelLabs/fuel-vm/pull/941): Fix heap memory reallocation after reset.
+
 ## [Version 0.59.2]
 
 ### Fixed

fuel-crypto/Cargo.toml

@@ -11,11 +11,13 @@ repository = { workspace = true }
 description = "Fuel cryptographic primitives."
 
 [dependencies]
+base64ct = { version = "=1.6.0" }
 coins-bip32 = { version = "0.8", default-features = false, optional = true }
 coins-bip39 = { version = "0.8", default-features = false, features = ["english"], optional = true }
 ecdsa = { version = "0.16", default-features = false }
 ed25519-dalek = { version = "2.0.0", default-features = false }
 fuel-types = { workspace = true, default-features = false }
+half = { version = "=2.4.1", default-features = false }
 k256 = { version = "0.13", default-features = false, features = ["digest", "ecdsa"] }
 lazy_static = { version = "1.4", optional = true }
 p256 = { version = "0.13", default-features = false, features = ["digest", "ecdsa"] }

fuel-crypto/src/lib.rs

@@ -19,6 +19,9 @@
 #[cfg(test)]
 use fuel_crypto as _;
 
+use base64ct as _;
+use half as _;
+
 /// Required export for using mnemonic keygen on [`SecretKey::new_from_mnemonic`]
 #[cfg(feature = "std")]
 #[doc(no_inline)]

fuel-vm/src/interpreter/memory.rs

@@ -51,6 +51,7 @@ mod tests;
 #[cfg(test)]
 mod impl_tests;
 
+#[allow(non_snake_case)]
 #[cfg(test)]
 mod allocation_tests;
 
@@ -187,6 +188,26 @@ impl MemoryInstance {
             self.heap[start..end].fill(0);
         } else {
             // Reallocation is needed.
+
+            // Need to clear dirty memory before expanding it. An example:
+            // Heap vector: [dirty, dirty, dirty, 0, 0, 0]
+            //                                   /|\
+            //                                    |
+            //                                   HP
+            //
+            // If we copy from [0, old_len), it means we copy the dirty memory as well.
+            // Ending up with:
+            // Heap vector: [0, 0, dirty, dirty, dirty, 0, 0, 0]
+            //              /|\
+            //               |
+            //              HP
+            //
+            // So, either we need to clear the memory before copying,
+            // or after we copied dirty parts.
+            // Clearing before looks like more readable solution.
+            let end = self.hp.saturating_sub(self.heap_offset());
+            self.heap[..end].fill(0);
+
             // To reduce frequent reallocations, allocate at least 256 bytes at once.
             // After that, double the allocation every time.
             let cap = new_len.next_power_of_two().clamp(256, MEM_SIZE);

fuel-vm/src/interpreter/memory/allocation_tests.rs

@@ -284,3 +284,56 @@ fn test_store_word(has_ownership: bool, a: Word, b: Word, c: Imm12) -> SimpleRes
 
     Ok(())
 }
+
+macro_rules! generate_test__memory_instance__grow_heap_by_after_reset__does_not_retain_dirty_memory_size {
+    ($size:expr) => {
+        paste::paste! {
+            #[test]
+            fn [<memory_instance__grow_heap_by_after_reset__does_not_retain_dirty_memory_size_ $size>]() -> SimpleResult<()> {
+                // given: a pre-initialized memory instance with $size bytes of heap memory
+                let mut memory_instance = MemoryInstance::new();
+                let sp = Reg::new(&10);
+                let mut hp = VM_MAX_RAM;
+                const SIZE: usize = $size;
+
+                memory_instance.grow_heap_by(sp, RegMut::new(&mut hp), SIZE.try_into().unwrap())?;
+                memory_instance.write_bytes_noownerchecks(
+                    memory_instance.hp,
+                    [1u8; SIZE],
+                )?;
+
+                // when: we reset and grow the heap again, triggering the reallocation
+                // after we grow the heap again, it should not have any memory left over from before
+                // the reset
+                memory_instance.reset();
+                let mut hp = VM_MAX_RAM;
+                const NEW_SIZE: usize = SIZE * 3;
+                memory_instance.grow_heap_by(
+                    sp,
+                    RegMut::new(&mut hp),
+                    NEW_SIZE.try_into().unwrap(),
+                )?;
+                let heap = memory_instance.heap_raw();
+                let heap_len = heap.len();
+
+                // then: we check that the heap is all zeroed out
+                assert_eq!(heap, vec![0u8; heap_len]);
+
+                Ok(())
+            }
+        }
+    };
+}
+
+// Generate tests with different sizes
+macro_rules! generate_tests__memory_instance__grow_heap_by_after_reset__does_not_retain_dirty_memory_size {
+    ($($size:expr),*) => {
+        $(
+            generate_test__memory_instance__grow_heap_by_after_reset__does_not_retain_dirty_memory_size!($size);
+        )*
+    };
+}
+
+generate_tests__memory_instance__grow_heap_by_after_reset__does_not_retain_dirty_memory_size!(
+    0, 1, 16, 32, 64, 128, 256, 512, 1024, 2048, 4096, 8192, 16384, 32768, 65536, 131072
+);