Merge pull request #17 from jgottula/fix/dont-use-strcpy-dammit

Implement overrun-safe C-string functions and replace usages of std::strcpy with them

src/openloco/envionment.cpp

@@ -5,6 +5,7 @@
 #include "interop/interop.hpp"
 #include "ui.h"
 #include "utility/collection.hpp"
+#include "utility/string.hpp"
 #include "platform/platform.h"
 
 using namespace openloco::interop;
@@ -110,12 +111,13 @@ namespace openloco::environment
         return result;
     }
 
-    static void set_directory(char * buffer, fs::path path)
+    template<size_t TSize, uint32_t TAddress>
+    static void set_directory(loco_global_array<char, TSize, TAddress> buffer, fs::path path)
     {
 #ifdef _OPENLOCO_USE_BOOST_FS_
-        std::strcpy(buffer, path.make_preferred().string().c_str());
+        utility::strcpy_safe(buffer, path.make_preferred().string().c_str());
 #else
-        std::strcpy(buffer, path.make_preferred().u8string().c_str());
+        utility::strcpy_safe(buffer, path.make_preferred().u8string().c_str());
 #endif
     }
 

src/openloco/interop/hooks.cpp

@@ -9,6 +9,7 @@
 #include "../station.h"
 #include "../things/vehicle.h"
 #include "../ui.h"
+#include "../utility/string.hpp"
 #include "../windowmgr.h"
 #include "interop.hpp"
 
@@ -224,9 +225,9 @@ fn_FindFirstFile(char *lpFileName, FindFileData *out)
     }
 
 #ifdef _OPENLOCO_USE_BOOST_FS_
-    strcpy(out->cFilename, data->fileList[0].filename().string().c_str());
+    utility::strcpy_safe(out->cFilename, data->fileList[0].filename().string().c_str());
 #else
-    strcpy(out->cFilename, data->fileList[0].filename().u8string().c_str());
+    utility::strcpy_safe(out->cFilename, data->fileList[0].filename().u8string().c_str());
 #endif
     data->fileList.erase(data->fileList.begin());
     return data;
@@ -244,9 +245,9 @@ fn_FindNextFile(Session *data, FindFileData *out)
     }
 
 #ifdef _OPENLOCO_USE_BOOST_FS_
-    strcpy(out->cFilename, data->fileList[0].filename().string().c_str());
+    utility::strcpy_safe(out->cFilename, data->fileList[0].filename().string().c_str());
 #else
-    strcpy(out->cFilename, data->fileList[0].filename().u8string().c_str());
+    utility::strcpy_safe(out->cFilename, data->fileList[0].filename().u8string().c_str());
 #endif
     data->fileList.erase(data->fileList.begin());
 
@@ -407,8 +408,10 @@ void openloco::interop::register_hooks()
             auto buffer = (char *)0x009D0D72;
             auto path = get_path((path_id)regs.ebx);
 #ifdef _OPENLOCO_USE_BOOST_FS_
+            // TODO: use utility::strlcpy with the buffer size instead of std::strcpy, if possible
             std::strcpy(buffer, path.make_preferred().string().c_str());
 #else
+            // TODO: use utility::strlcpy with the buffer size instead of std::strcpy, if possible
             std::strcpy(buffer, path.make_preferred().u8string().c_str());
 #endif
             regs.ebx = (int32_t)buffer;

src/openloco/utility/string.hpp

@@ -1,6 +1,8 @@
 #pragma once
 
+#include <algorithm>
 #include <cctype>
+#include <cstring>
 #include <string>
 
 namespace openloco::utility
@@ -23,4 +25,90 @@ namespace openloco::utility
         }
         return true;
     }
+
+    inline size_t strlcpy(char *dest, const char *src, size_t size)
+    {
+        size_t src_len = std::strlen(src);
+
+        if (src_len < size)
+        {
+            std::memcpy(dest, src, src_len + 1);
+        }
+        else
+        {
+            std::memcpy(dest, src, size);
+            dest[size - 1] = '\0';
+        }
+
+        return src_len;
+    }
+
+    inline size_t strlcat(char *dest, const char *src, size_t size)
+    {
+        size_t src_len = std::strlen(src);
+
+        if (size == 0)
+        {
+            return src_len;
+        }
+
+        // this lambda is essentially a reimplementation of strnlen, which isn't standard
+        size_t dest_len = [=]
+        {
+            auto dest_end = reinterpret_cast<const char *>(std::memchr(dest, '\0', size));
+            if (dest_end != nullptr)
+            {
+                return static_cast<size_t>(dest_end - dest);
+            }
+            else
+            {
+                return size;
+            }
+        }();
+
+        if (dest_len < size)
+        {
+            size_t copy_count = std::min<size_t>((size - dest_len) - 1, src_len);
+            char  *copy_ptr   = (dest + dest_len);
+
+            std::memcpy(copy_ptr, src, copy_count);
+            copy_ptr[copy_count] = '\0';
+        }
+
+        return (dest_len + src_len);
+    }
+
+    template<size_t N>
+    inline void strcpy_safe(char (&dest)[N], const char *src)
+    {
+        (void)strlcpy(dest, src, N);
+    }
+
+    template<size_t N>
+    inline void strcat_safe(char (&dest)[N], const char *src)
+    {
+        (void)strlcat(dest, src, N);
+    }
+
+    template<size_t N, typename... Args>
+    inline int sprintf_safe(char (&dest)[N], const char *fmt, Args&&... args)
+    {
+        return snprintf(dest, N, fmt, std::forward<Args>(args)...);
+    }
+
+    // intended for use with e.g. loco_global_array<char, N, A>
+    template<typename T>
+    inline auto strcpy_safe(T& dest, const char *src)
+        -> std::enable_if_t<std::is_convertible_v<T, char *> && std::is_member_function_pointer_v<decltype(&T::size)>, void>
+    {
+        (void)strlcpy(dest, src, dest.size());
+    }
+
+    // intended for use with e.g. loco_global_array<char, N, A>
+    template<typename T>
+    inline auto strcat_safe(T& dest, const char *src)
+        -> std::enable_if_t<std::is_convertible_v<T, char *> && std::is_member_function_pointer_v<decltype(&T::size)>, void>
+    {
+        (void)strlcat(dest, src, dest.size());
+    }
 }

src/openloco/windows/promptbrowsewnd.cpp

@@ -70,15 +70,15 @@ namespace openloco::ui::windows
         {
             _fileType = (uint8_t)browse_file_type::landscape;
         }
-        std::strcpy(_title, title);
-        std::strcpy(_filter, filter);
+        utility::strcpy_safe(_title, title);
+        utility::strcpy_safe(_filter, filter);
 
 #ifdef _OPENLOCO_USE_BOOST_FS_
-        std::strcpy(_directory, directory.make_preferred().string().c_str());
+        utility::strcpy_safe(_directory, directory.make_preferred().string().c_str());
 #else
-        std::strcpy(_directory, directory.make_preferred().u8string().c_str());
+        utility::strcpy_safe(_directory, directory.make_preferred().u8string().c_str());
 #endif
-        std::strcpy(_text_input_buffer, baseName.c_str());
+        utility::strcpy_safe(_text_input_buffer, baseName.c_str());
 
         sub_446A93();
         auto window = windowmgr::create_window_centred(window_type::prompt_browse, 500, 380, 0x1202, (void *)0x004FB308);
@@ -110,6 +110,7 @@ namespace openloco::ui::windows
                     return windowmgr::find(window_type::prompt_browse) != nullptr;
                 });
             windowmgr::current_modal_type(window_type::undefined);
+            // TODO: use utility::strlcpy with the buffer size instead of std::strcpy, if possible
             std::strcpy(szPath, _directory);
             if (szPath[0] != '\0')
             {