diff --git a/loader/loader.c b/loader/loader.c index d6fcb0d67..1f3f8536a 100644 --- a/loader/loader.c +++ b/loader/loader.c @@ -743,8 +743,8 @@ uint32_t loader_parse_version_string(char *vers_str) { return VK_MAKE_API_VERSION(variant, major, minor, patch); } -bool compare_vk_extension_properties(const VkExtensionProperties *op1, const VkExtensionProperties *op2) { - return strcmp(op1->extensionName, op2->extensionName) == 0 ? true : false; +TEST_FUNCTION_EXPORT bool compare_vk_extension_properties(const VkExtensionProperties *op1, const VkExtensionProperties *op2) { + return strncmp(op1->extensionName, op2->extensionName, VK_MAX_EXTENSION_NAME_SIZE) == 0 ? true : false; } // Search the given ext_array for an extension matching the given vk_ext_prop diff --git a/loader/loader.h b/loader/loader.h index 6f6385152..cebadfe81 100644 --- a/loader/loader.h +++ b/loader/loader.h @@ -82,7 +82,7 @@ extern struct loader_struct loader; extern loader_platform_thread_mutex loader_lock; extern loader_platform_thread_mutex loader_preload_icd_lock; -bool compare_vk_extension_properties(const VkExtensionProperties *op1, const VkExtensionProperties *op2); +TEST_FUNCTION_EXPORT bool compare_vk_extension_properties(const VkExtensionProperties *op1, const VkExtensionProperties *op2); VkResult loader_validate_layers(const struct loader_instance *inst, const uint32_t layer_count, const char *const *ppEnabledLayerNames, const struct loader_layer_list *list); diff --git a/tests/loader_fuzz_tests.cpp b/tests/loader_fuzz_tests.cpp index 6fa5b2572..45e0b4940 100644 --- a/tests/loader_fuzz_tests.cpp +++ b/tests/loader_fuzz_tests.cpp @@ -27,6 +27,8 @@ #include "framework/test_environment.h" +#include +#include #include #include @@ -389,3 +391,28 @@ TEST(JsonStringPrint, PrintPreallocatedTerminatesAtRealEnd) { loader_cJSON_Delete(json); std::filesystem::remove(json_path); } + +// extensionName is a fixed VK_MAX_EXTENSION_NAME_SIZE array which the loader copies verbatim from a driver's +// VkExtensionProperties. A non-conformant ICD can fill all VK_MAX_EXTENSION_NAME_SIZE bytes with no null +// terminator; comparing two such names with an unbounded strcmp reads past the end of the array (and, at the +// end of a list, past the allocation). compare_vk_extension_properties must bound the compare to the field. +TEST(ExtensionNameCompare, UnterminatedNameDoesNotReadPastField) { + // Allocate each VkExtensionProperties in its own exact-sized block so an over-read runs into the ASAN redzone. + VkExtensionProperties* a = static_cast(malloc(sizeof(VkExtensionProperties))); + VkExtensionProperties* b = static_cast(malloc(sizeof(VkExtensionProperties))); + ASSERT_NE(a, nullptr); + ASSERT_NE(b, nullptr); + // Fill every byte (extensionName and specVersion) with a non-null value: no terminator anywhere in the object. + memset(a, 'A', sizeof(VkExtensionProperties)); + memset(b, 'A', sizeof(VkExtensionProperties)); + + // Equal within the field: must report a match without reading past VK_MAX_EXTENSION_NAME_SIZE. + EXPECT_TRUE(compare_vk_extension_properties(a, b)); + + // Differ inside the field: must report no match, again without over-reading. + b->extensionName[VK_MAX_EXTENSION_NAME_SIZE - 1] = 'B'; + EXPECT_FALSE(compare_vk_extension_properties(a, b)); + + free(a); + free(b); +}