From 2de1f81c7209bf8206d5ef622c326e1a32379a82 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 30 May 2019 14:43:45 +0200 Subject: [PATCH 1/3] loader: Rename strcpy_s() implementation to strcpy_sx() The implementation strcpy_s() in loader is wrong (to be fixed) and limited in comparison to strcpy_s() spec. Rename it to strcpy_sx() to indicate it is not the one from standard library. --- lib/loader/loader.c | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/lib/loader/loader.c b/lib/loader/loader.c index 935d30c3d..a2815aff5 100644 --- a/lib/loader/loader.c +++ b/lib/loader/loader.c @@ -42,12 +42,16 @@ #define ATTR_FORMAT(...) #endif -#if !_WIN32 +#if _WIN32 +#define strcpy_sx strcpy_s +#else /* - * Provide strcpy_s() for GNU libc. + * Limited variant of strcpy_s(). + * + * Provided for C standard libraries where strcpy_s() is not available. * The availability check might need to adjusted for other C standard library implementations. */ -static void strcpy_s(char* dest, size_t destsz, const char* src) +static void strcpy_sx(char* restrict dest, size_t destsz, const char* restrict src) { size_t len = strlen(src); if (len > destsz - 1) @@ -124,7 +128,7 @@ evmc_create_fn evmc_load(const char* filename, enum evmc_loader_error_code* erro const char prefix[] = "evmc_create_"; const size_t prefix_length = strlen(prefix); char prefixed_name[sizeof(prefix) + PATH_MAX_LENGTH]; - strcpy_s(prefixed_name, sizeof(prefixed_name), prefix); + strcpy_sx(prefixed_name, sizeof(prefixed_name), prefix); // Find filename in the path. const char* sep_pos = strrchr(filename, '/'); @@ -142,7 +146,7 @@ evmc_create_fn evmc_load(const char* filename, enum evmc_loader_error_code* erro name_pos += lib_prefix_length; char* base_name = prefixed_name + prefix_length; - strcpy_s(base_name, PATH_MAX_LENGTH, name_pos); + strcpy_sx(base_name, PATH_MAX_LENGTH, name_pos); // Trim the file extension. char* ext_pos = strrchr(prefixed_name, '.'); From 5757da58596d03a98f0c2f9f0baa86fa49b55a36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 30 May 2019 15:30:30 +0200 Subject: [PATCH 2/3] loader: Expose strcpy_sx() for testing --- lib/loader/loader.c | 6 +++++- test/unittests/test_loader.cpp | 18 ++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/lib/loader/loader.c b/lib/loader/loader.c index a2815aff5..b81d22ed3 100644 --- a/lib/loader/loader.c +++ b/lib/loader/loader.c @@ -51,7 +51,11 @@ * Provided for C standard libraries where strcpy_s() is not available. * The availability check might need to adjusted for other C standard library implementations. */ -static void strcpy_sx(char* restrict dest, size_t destsz, const char* restrict src) +#if !defined(EVMC_LOADER_MOCK) +static +#endif + void + strcpy_sx(char* restrict dest, size_t destsz, const char* restrict src) { size_t len = strlen(src); if (len > destsz - 1) diff --git a/test/unittests/test_loader.cpp b/test/unittests/test_loader.cpp index bdde22e98..9f73ae989 100644 --- a/test/unittests/test_loader.cpp +++ b/test/unittests/test_loader.cpp @@ -16,6 +16,13 @@ static constexpr bool is_windows = false; #endif extern "C" { +#if _WIN32 +#define strcpy_sx strcpy_s +#else +/// Declaration of internal function defined in loader.c. +void strcpy_sx(char* dest, size_t destsz, const char* src); +#endif + /// The library path expected by mocked evmc_test_load_library(). extern const char* evmc_test_library_path; @@ -118,6 +125,17 @@ static evmc_instance* create_failure() return nullptr; } +TEST_F(loader, strcpy_sx) +{ + const auto input = "12"; + char buf[2] = {0x0f, 0x0e}; + static_assert(sizeof(input) > sizeof(buf), ""); + + strcpy_sx(buf, sizeof(buf), input); + EXPECT_EQ(buf[0], 0); + EXPECT_EQ(buf[1], 0); +} + TEST_F(loader, load_nonexistent) { constexpr auto path = "nonexistent"; From 8c3b3e61e6ede5d7b3c27298e72270171a0dc13d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Bylica?= Date: Thu, 30 May 2019 15:48:38 +0200 Subject: [PATCH 3/3] loader: Change strcpy_sx() to be limited version of strcpy_s() The strcpy_sx() now reports errors and behaves as strcpy_s() when buffer is too small (nulls the buffer). But it does not have all the checks. --- lib/loader/loader.c | 13 ++++++++++--- test/unittests/test_loader.cpp | 23 +++++++++++++++++------ 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/lib/loader/loader.c b/lib/loader/loader.c index b81d22ed3..72b2afaac 100644 --- a/lib/loader/loader.c +++ b/lib/loader/loader.c @@ -54,14 +54,21 @@ #if !defined(EVMC_LOADER_MOCK) static #endif - void + int strcpy_sx(char* restrict dest, size_t destsz, const char* restrict src) { size_t len = strlen(src); - if (len > destsz - 1) - len = destsz - 1; + if (len >= destsz) + { + // The input src will not fit into the dest buffer. + // Set the first byte of the dest to null to make it effectively empty string + // and return error. + dest[0] = 0; + return 1; + } memcpy(dest, src, len); dest[len] = 0; + return 0; } #endif diff --git a/test/unittests/test_loader.cpp b/test/unittests/test_loader.cpp index 9f73ae989..010cbd685 100644 --- a/test/unittests/test_loader.cpp +++ b/test/unittests/test_loader.cpp @@ -20,7 +20,7 @@ extern "C" { #define strcpy_sx strcpy_s #else /// Declaration of internal function defined in loader.c. -void strcpy_sx(char* dest, size_t destsz, const char* src); +int strcpy_sx(char* dest, size_t destsz, const char* src); #endif /// The library path expected by mocked evmc_test_load_library(). @@ -127,13 +127,24 @@ static evmc_instance* create_failure() TEST_F(loader, strcpy_sx) { - const auto input = "12"; - char buf[2] = {0x0f, 0x0e}; - static_assert(sizeof(input) > sizeof(buf), ""); - - strcpy_sx(buf, sizeof(buf), input); + const char input_empty[] = ""; + const char input_that_fits[] = "x"; + const char input_too_big[] = "12"; + char buf[2] = {0x0f, 0x0f}; + static_assert(sizeof(input_empty) <= sizeof(buf), ""); + static_assert(sizeof(input_that_fits) <= sizeof(buf), ""); + static_assert(sizeof(input_too_big) > sizeof(buf), ""); + + EXPECT_EQ(strcpy_sx(buf, sizeof(buf), input_empty), 0); EXPECT_EQ(buf[0], 0); + EXPECT_EQ(buf[1], 0x0f); + + EXPECT_EQ(strcpy_sx(buf, sizeof(buf), input_that_fits), 0); + EXPECT_EQ(buf[0], 'x'); EXPECT_EQ(buf[1], 0); + + EXPECT_NE(strcpy_sx(buf, sizeof(buf), input_too_big), 0); + EXPECT_EQ(buf[0], 0); } TEST_F(loader, load_nonexistent)