Merge pull request #809 from wasmx/only-memory-size-helper

Fix overflows with memory of max size
wasmx · Jan 13, 2022 · cab8a7b · cab8a7b
2 parents 95d5087 + 20cb1a0
commit cab8a7b
Show file tree

Hide file tree

Showing 4 changed files with 31 additions and 10 deletions.
diff --git a/lib/fizzy/execute.cpp b/lib/fizzy/execute.cpp
@@ -145,17 +145,19 @@ inline uint32_t grow_memory(
     assert(memory_pages_limit <= MaxMemoryPagesLimit);
     assert(cur_pages <= memory_pages_limit);
 
-    const auto new_pages = uint64_t{cur_pages} + delta_pages;
-    if (new_pages > memory_pages_limit)
+    const auto new_pages_u64 = uint64_t{cur_pages} + delta_pages;
+    if (new_pages_u64 > memory_pages_limit)
         return static_cast<uint32_t>(-1);
 
+    const auto new_pages = static_cast<uint32_t>(new_pages_u64);
+
     try
     {
-        // new_pages <= memory_pages_limit <= MaxMemoryPagesLimit guarantees multiplication
+        // new_pages <= memory_pages_limit <= MaxMemoryPagesLimit guarantees memory_pages_to_bytes
         // will not overflow uint32_t.
-        assert(new_pages * PageSize <= std::numeric_limits<uint32_t>::max());
+        assert(memory_pages_to_bytes(new_pages) <= std::numeric_limits<uint32_t>::max());
         static_assert(sizeof(size_t) >= sizeof(uint32_t));
-        memory.resize(static_cast<size_t>(new_pages) * PageSize);
+        memory.resize(static_cast<size_t>(memory_pages_to_bytes(new_pages)));
         return static_cast<uint32_t>(cur_pages);
     }
     catch (...)

diff --git a/lib/fizzy/instantiate.cpp b/lib/fizzy/instantiate.cpp
@@ -121,7 +121,8 @@ void match_imported_memories(const std::vector<Memory>& module_imported_memories
 
         const auto min = imported_memories[0].limits.min;
         const auto& max = imported_memories[0].limits.max;
-        if (size < min * PageSize || (max.has_value() && size > *max * PageSize))
+        if (size < memory_pages_to_bytes(min) ||
+            (max.has_value() && size > memory_pages_to_bytes(*max)))
             throw instantiate_error{"provided imported memory doesn't fit provided limits"};
     }
 }
@@ -183,7 +184,7 @@ std::tuple<bytes_ptr, Limits> allocate_memory(const std::vector<Memory>& module_
     if (memory_pages_limit > MaxMemoryPagesLimit)
     {
         throw instantiate_error{"hard memory limit cannot exceed " +
-                                std::to_string(uint64_t{MaxMemoryPagesLimit} * PageSize) +
+                                std::to_string(memory_pages_to_bytes(MaxMemoryPagesLimit)) +
                                 " bytes"};
     }
 
@@ -199,11 +200,13 @@ std::tuple<bytes_ptr, Limits> allocate_memory(const std::vector<Memory>& module_
             (memory_max.has_value() && *memory_max > memory_pages_limit))
         {
             throw instantiate_error{"cannot exceed hard memory limit of " +
-                                    std::to_string(memory_pages_limit * PageSize) + " bytes"};
+                                    std::to_string(memory_pages_to_bytes(memory_pages_limit)) +
+                                    " bytes"};
         }
 
         // NOTE: fill it with zeroes
-        bytes_ptr memory{new bytes(memory_min * PageSize, 0), bytes_delete};
+        bytes_ptr memory{
+            new bytes(static_cast<size_t>(memory_pages_to_bytes(memory_min)), 0), bytes_delete};
         return {std::move(memory), module_memories[0].limits};
     }
     else if (imported_memories.size() == 1)
@@ -216,7 +219,8 @@ std::tuple<bytes_ptr, Limits> allocate_memory(const std::vector<Memory>& module_
             (memory_max.has_value() && *memory_max > memory_pages_limit))
         {
             throw instantiate_error{"imported memory limits cannot exceed hard memory limit of " +
-                                    std::to_string(memory_pages_limit * PageSize) + " bytes"};
+                                    std::to_string(memory_pages_to_bytes(memory_pages_limit)) +
+                                    " bytes"};
         }
 
         bytes_ptr memory{imported_memories[0].data, null_delete};

diff --git a/lib/fizzy/limits.hpp b/lib/fizzy/limits.hpp
@@ -11,6 +11,12 @@ namespace fizzy
 /// The page size as defined by the WebAssembly 1.0 specification.
 constexpr uint32_t PageSize = 65536;
 
+/// Convert memory size in pages to size in bytes.
+inline constexpr uint64_t memory_pages_to_bytes(uint32_t pages) noexcept
+{
+    return uint64_t{pages} * PageSize;
+}
+
 /// The maximum memory page limit as defined by the specification.
 /// It is only possible to address 4 GB (32-bit) of memory.
 constexpr uint32_t MaxMemoryPagesLimit = (4 * 1024 * 1024 * 1024ULL) / PageSize;

diff --git a/test/unittests/instantiate_test.cpp b/test/unittests/instantiate_test.cpp
@@ -579,6 +579,15 @@ TEST(instantiate, imported_memory_custom_hard_limit)
     EXPECT_NO_THROW(instantiate(*module_max_limit, {}, {}, {{&memory, {3, 6}}}, {}, 8));
 }
 
+TEST(instantiate, memory_pages_to_bytes)
+{
+    EXPECT_EQ(memory_pages_to_bytes(0), 0);
+    EXPECT_EQ(memory_pages_to_bytes(1), 65536);
+    EXPECT_EQ(memory_pages_to_bytes(2), 2 * 65536);
+    EXPECT_EQ(memory_pages_to_bytes(65536), uint64_t{65536} * 65536);
+    EXPECT_EQ(memory_pages_to_bytes(MaxMemoryPagesLimit), 4 * 1024 * 1024 * 1024ULL);
+}
+
 TEST(instantiate, element_section)
 {
     /* wat2wasm