From 77cfaf43ae23b794cea95ed35c2dc1660365ef61 Mon Sep 17 00:00:00 2001 From: Ivan Mogilko Date: Thu, 7 Nov 2024 01:47:33 +0300 Subject: [PATCH] Engine: use ags_strncpy_s instead of snprintf where truncation expected --- Common/game/main_game_file.cpp | 9 ++++++--- Common/util/string_compat.c | 28 ++++++++++++++++++++++++++++ Common/util/string_compat.h | 1 + Engine/ac/sys_events.cpp | 3 ++- Engine/main/update.cpp | 2 +- 5 files changed, 38 insertions(+), 5 deletions(-) diff --git a/Common/game/main_game_file.cpp b/Common/game/main_game_file.cpp index bd97c13888..9c780ad4ec 100644 --- a/Common/game/main_game_file.cpp +++ b/Common/game/main_game_file.cpp @@ -639,16 +639,19 @@ void UpgradeCharacters(GameSetupStruct &game, GameDataVersion data_ver) } // Fixup character script names for 2.x (EGO -> cEgo) + // In 2.x versions the "scriptname" field in game data contained a name + // limited by 14 chars (although serialized in 20 bytes). After reading, + // it was exported as "cScriptname..." for the script. if (data_ver <= kGameVersion_272) { - char namelwr[LEGACY_MAX_SCRIPT_NAME_LEN]; + char namelwr[LEGACY_MAX_SCRIPT_NAME_LEN - 1]; for (int i = 0; i < numcharacters; i++) { if (chars[i].scrname[0] == 0) continue; - memcpy(namelwr, chars[i].scrname, LEGACY_MAX_SCRIPT_NAME_LEN); + ags_strncpy_s(namelwr, sizeof(namelwr), chars[i].scrname, LEGACY_MAX_SCRIPT_NAME_LEN - 2); ags_strlwr(namelwr + 1); // lowercase starting with the second char - snprintf(chars[i].scrname, LEGACY_MAX_SCRIPT_NAME_LEN, "c%s", namelwr); + snprintf(chars[i].scrname, sizeof(chars[i].scrname), "c%s", namelwr); chars2[i].scrname_new = chars[i].scrname; } } diff --git a/Common/util/string_compat.c b/Common/util/string_compat.c index c012c93b0a..06a2ea1aff 100644 --- a/Common/util/string_compat.c +++ b/Common/util/string_compat.c @@ -13,9 +13,11 @@ //============================================================================= #include "util/string_compat.h" #include +#include #include #include #include "core/platform.h" +#include "debug/assert.h" char *ags_strlwr(char *s) { @@ -63,3 +65,29 @@ char *ags_strstr(const char *haystack, const char *needle) { return strstr(haystack, needle); } + +int ags_strncpy_s(char *dest, size_t dest_sz, const char *src, size_t count) +{ + // NOTE: implementation approximately mimics explanation for "strncpy_s": + // https://en.cppreference.com/w/c/string/byte/strncpy + assert(dest && dest_sz > 0 && ((dest + dest_sz - 1 < src) || (dest > src + count))); + if (!dest || dest_sz == 0 || ((dest <= src) && (dest + dest_sz - 1 >= src)) || ((src <= dest) && (src + count - 1 >= dest))) + return EINVAL; // null buffer, or dest and src overlap + if (!src) + { + dest[0] = 0; // ensure null terminator + return EINVAL; + } + + const size_t copy_len = (count < dest_sz - 1) ? count : dest_sz - 1; // reserve null-terminator + const char *psrc = src; + const char *src_end = src + copy_len; + char *pdst = dest; + for (; *psrc && (psrc != src_end); ++psrc, ++pdst) + *pdst = *psrc; + *pdst = 0; // ensure null terminator + assert((*psrc == 0) || ((psrc - src) == count)); // assert that no *unintended* truncation occured + if ((*psrc != 0) && ((psrc - src) < count)) + return ERANGE; // not enough dest buffer - error + return 0; // success +} diff --git a/Common/util/string_compat.h b/Common/util/string_compat.h index 78664f1c48..257b3c5292 100644 --- a/Common/util/string_compat.h +++ b/Common/util/string_compat.h @@ -26,6 +26,7 @@ int ags_stricmp(const char *, const char *); int ags_strnicmp(const char *, const char *, size_t); char *ags_strdup(const char *s); char *ags_strstr(const char *haystack, const char *needle); +int ags_strncpy_s(char *dest, size_t dest_sz, const char *src, size_t count); #ifdef __cplusplus } diff --git a/Engine/ac/sys_events.cpp b/Engine/ac/sys_events.cpp index 3918d7ec37..65b9263c5e 100644 --- a/Engine/ac/sys_events.cpp +++ b/Engine/ac/sys_events.cpp @@ -28,6 +28,7 @@ #include "platform/base/agsplatformdriver.h" #include "platform/base/sys_main.h" #include "main/engine.h" +#include "util/string_compat.h" #include "util/string_utils.h" #include "util/utf8.h" @@ -64,7 +65,7 @@ KeyInput sdl_keyevt_to_ags_key(const SDL_Event &event, bool old_keyhandle) ki.CompatKey = ki.Key; } } - snprintf(ki.Text, KeyInput::UTF8_ARR_SIZE, "%s", event.text.text); + ags_strncpy_s(ki.Text, KeyInput::UTF8_ARR_SIZE, event.text.text, KeyInput::UTF8_ARR_SIZE - 1); Utf8::GetChar(event.text.text, sizeof(SDL_TextInputEvent::text), &ki.UChar); return ki; case SDL_KEYDOWN: diff --git a/Engine/main/update.cpp b/Engine/main/update.cpp index 980c2b4634..193bdc1c46 100644 --- a/Engine/main/update.cpp +++ b/Engine/main/update.cpp @@ -92,7 +92,7 @@ static void movelist_handle_targetfix(const fixed xpermove, const fixed ypermove else if ((ypermove & 0xffff0000) == 0) targety -= tfix; // Y per move is -1 exactly, don't snap to finish - else if (ypermove == 0xffff0000) {} + else if ((ypermove & 0xffffffff) == 0xffff0000) {} // Y per move is > -1, so finish the move else if ((ypermove & 0xffff0000) == 0xffff0000) targety += tfix;