Engine: use ags_strncpy_s instead of snprintf where truncation expected

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;
         }
     }

Common/util/string_compat.c

@@ -13,9 +13,11 @@
 //=============================================================================
 #include "util/string_compat.h"
 #include <ctype.h>
+#include <errno.h>
 #include <stdlib.h>
 #include <string.h>
 #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
+}

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
 }

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:

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;