Fix off-by-one overflow in windows_get_registry_files()

loader/loader_windows.c

@@ -462,7 +462,8 @@ VkResult windows_get_registry_files(const struct loader_instance *inst, char *lo
                             goto out;
                         }
                         *reg_data[0] = '\0';
-                    } else if (strlen(*reg_data) + name_size + 1 > *reg_data_size) {
+                        // Reserve room for the existing contents, a PATH_SEPARATOR, the new value, and the null terminator.
+                    } else if (strlen(*reg_data) + name_size + 2 > *reg_data_size) {
                         void *new_ptr = loader_instance_heap_realloc(inst, *reg_data, *reg_data_size, *reg_data_size * 2,
                                                                      VK_SYSTEM_ALLOCATION_SCOPE_INSTANCE);
                         if (NULL == new_ptr) {
@@ -534,7 +535,8 @@ VkResult windows_get_registry_files(const struct loader_instance *inst, char *lo
 
                     if (strlen(*reg_data) == 0) {
                         // The list is emtpy. Add the first entry.
-                        (void)snprintf(*reg_data, name_size + 1, "%s", name);
+                        // Bound the write by the actual destination size, not the source length.
+                        (void)snprintf(*reg_data, *reg_data_size, "%s", name);
                         found = true;
                     } else {
                         // At this point the reg_data variable contains other JSON paths, likely from the PNP/device section
@@ -554,7 +556,10 @@ VkResult windows_get_registry_files(const struct loader_instance *inst, char *lo
                         // Only skip if we are adding a driver and a duplicate was found
                         if (!is_driver || (is_driver && foundDuplicate == false)) {
                             // Add the new entry to the list.
-                            (void)snprintf(*reg_data + strlen(*reg_data), name_size + 2, "%c%s", PATH_SEPARATOR, name);
+                            // Bound the write by the remaining destination size, not the source length, so the
+                            // PATH_SEPARATOR + name + null terminator can never spill past the allocation.
+                            size_t cur_len = strlen(*reg_data);
+                            (void)snprintf(*reg_data + cur_len, *reg_data_size - cur_len, "%c%s", PATH_SEPARATOR, name);
                             found = true;
                         } else {
                             loader_log(

tests/framework/shim/windows_shim.cpp

@@ -337,12 +337,15 @@ LSTATUS __stdcall ShimRegEnumValueA(HKEY hKey, DWORD dwIndex, LPSTR lpValueName,
     if (dwIndex >= location.size()) return ERROR_NO_MORE_ITEMS;
 
     std::string name = narrow(location[dwIndex].name);
-    if (*lpcchValueName < name.size()) return ERROR_NO_MORE_ITEMS;
+    // The destination buffer must hold the name plus a null terminator, matching the real RegEnumValueA contract.
+    if (*lpcchValueName <= name.size()) return ERROR_NO_MORE_ITEMS;
     for (size_t i = 0; i < name.size(); i++) {
         lpValueName[i] = name[i];
     }
     lpValueName[name.size()] = '\0';
-    *lpcchValueName = static_cast<DWORD>(name.size() + 1);
+    // RegEnumValueA reports the number of characters in the name *excluding* the null terminator. Reporting the
+    // length including the terminator (off by one) hides destination-buffer-sizing bugs in callers, so match the API.
+    *lpcchValueName = static_cast<DWORD>(name.size());
     if (*lpcbData < sizeof(DWORD)) return ERROR_NO_MORE_ITEMS;
     DWORD *lpcbData_dword = reinterpret_cast<DWORD *>(lpData);
     *lpcbData_dword = location[dwIndex].value;

tests/loader_regression_tests.cpp

@@ -4361,6 +4361,39 @@ TEST(RegistryManifestParsing, OversizedD3DKMTDriverPathDoesNotOverflow) {
     InstWrapper inst{env.vulkan_functions};
     inst.CheckCreate(VK_ERROR_INCOMPATIBLE_DRIVER);
 }
+
+// Regression test for an off-by-one heap buffer overflow in windows_get_registry_files(). That function accumulates
+// manifest paths enumerated from the generic Khronos\Vulkan registry locations into a heap buffer that starts at 4096
+// bytes and grows by doubling. Each appended entry is written as PATH_SEPARATOR + name + null terminator, but the
+// grow check only reserved name + null (it forgot the separator). When the running length lined up so that
+// strlen(reg_data) + name_size + 1 == reg_data_size exactly, the buffer was not grown and the terminating null byte
+// was written one element past the end of the allocation.
+//
+// The registry value names below are sized so the third append lands the write exactly on the 4096-byte boundary:
+//   entry 0:            strlen -> 2000
+//   entry 1: +sep+name  strlen -> 2000 + 1 + 2000 = 4001
+//   entry 2: +sep+name  writes 1 + 94 + 1 = 96 bytes at offset 4001 -> last index 4096 (one past a 4096-byte buffer)
+// 2000 + 2000 + 94 == 4094, and 4001 + 94 + 1 == 4096, so the unpatched grow check (which compares against
+// name_size + 1) does not trigger a realloc and the write overflows by a single null byte. Run under ASAN this
+// aborts before the fix; with the fix the buffer is grown to fit and instance creation completes cleanly.
+TEST(RegistryManifestParsing, AppendAtBufferBoundaryDoesNotOverflow) {
+    FrameworkEnvironment env{};
+    env.add_icd(TEST_ICD_PATH_VERSION_2).add_physical_device("physical_device_0");
+
+    auto make_named_path = [](size_t total_len) {
+        const std::string suffix = ".json";
+        return std::filesystem::path(std::string(total_len - suffix.size(), 'a') + suffix);
+    };
+
+    env.platform_shim->add_manifest_to_registry(ManifestCategory::explicit_layer, make_named_path(2000));
+    env.platform_shim->add_manifest_to_registry(ManifestCategory::explicit_layer, make_named_path(2000));
+    env.platform_shim->add_manifest_to_registry(ManifestCategory::explicit_layer, make_named_path(94));
+
+    // None of the registry entries point at a real manifest, so no layers are added - the loader simply needs to
+    // enumerate the (bogus) paths without overflowing its accumulation buffer.
+    InstWrapper inst{env.vulkan_functions};
+    inst.CheckCreate();
+}
 #endif
 
 TEST(LibraryLoading, SystemLocations) {