From 5b1dd64f5090ebf7c3f05ce630efbc7066acdc33 Mon Sep 17 00:00:00 2001
From: Aidan MacDonald <amachronic@protonmail.com>
Date: Tue, 29 Nov 2022 20:12:46 +0000
Subject: [PATCH] settings: Add helper function for handling filename settings

The old inline implementation was buggy -- it didn't check the
suffix was actually at the end of the string before stripping
it, and didn't check for truncation. This version also avoids
using an extra buffer.

Change-Id: I33abfefc508675d3079cc64a9ad2b11d646baa0d
---
 apps/settings.c | 72 +++++++++++++++++++++++++++++++++----------------
 apps/settings.h |  3 +++
 2 files changed, 52 insertions(+), 23 deletions(-)

diff --git a/apps/settings.c b/apps/settings.c
index 3d2c463cdd..cd93dcfb4b 100644
--- a/apps/settings.c
+++ b/apps/settings.c
@@ -262,6 +262,52 @@ bool cfg_string_to_int(int setting_id, int* out, const char* str)
     return false;
 }
 
+/**
+ * Copy an input string to an output buffer, stripping the prefix and
+ * suffix listed in the filename setting. Returns false if the output
+ * string does not fit in the buffer or is longer than the setting's
+ * max_len, and the output buffer will not be modified.
+ *
+ * Returns true if the setting was copied successfully. The input and
+ * output buffers are allowed to alias.
+ */
+bool copy_filename_setting(char *buf, size_t buflen, const char *input,
+                           const struct filename_setting *fs)
+{
+    size_t input_len = strlen(input);
+    size_t len;
+
+    if (fs->prefix)
+    {
+        len = strlen(fs->prefix);
+        if (len <= input_len && !strncasecmp(input, fs->prefix, len))
+        {
+            input += len;
+            input_len -= len;
+        }
+    }
+
+    if (fs->suffix)
+    {
+        len = strlen(fs->suffix);
+        if (len <= input_len &&
+            !strcasecmp(input + input_len - len, fs->suffix))
+        {
+            input_len -= len;
+        }
+    }
+
+    /* Make sure it fits the output buffer and repsects the setting's max_len.
+     * Note that max_len is a buffer size and thus includes a null terminator */
+    if (input_len >= (size_t)fs->max_len || input_len >= buflen)
+        return false;
+
+    /* Copy what remains into buf - use memmove in case of aliasing */
+    memmove(buf, input, input_len);
+    buf[input_len] = '\0';
+    return true;
+}
+
 bool settings_load_config(const char* file, bool apply)
 {
     int fd;
@@ -335,29 +381,9 @@ bool settings_load_config(const char* file, bool apply)
                     case F_T_CHARPTR:
                     case F_T_UCHARPTR:
                     {
-                        char storage[MAX_PATH];
-                        if (settings[i].filename_setting->prefix)
-                        {
-                            const char *dir = settings[i].filename_setting->prefix;
-                            size_t len = strlen(dir);
-                            if (!strncasecmp(value, dir, len))
-                            {
-                                strmemccpy(storage, &value[len], MAX_PATH);
-                            }
-                            else
-                                strmemccpy(storage, value, MAX_PATH);
-
-                        }
-                        else
-                            strmemccpy(storage, value, MAX_PATH);
-
-                        if (settings[i].filename_setting->suffix)
-                        {
-                            char *s = strcasestr(storage,settings[i].filename_setting->suffix);
-                            if (s) *s = '\0';
-                        }
-                        strmemccpy((char*)settings[i].setting, storage,
-                                   settings[i].filename_setting->max_len);
+                        const struct filename_setting *fs = settings[i].filename_setting;
+                        copy_filename_setting((char*)settings[i].setting,
+                                              fs->max_len, value, fs);
                         break;
                     }
                 }
diff --git a/apps/settings.h b/apps/settings.h
index 37e546f6fc..9c5bf0470f 100644
--- a/apps/settings.h
+++ b/apps/settings.h
@@ -247,6 +247,7 @@ enum {
 bool settings_save_config(int options);
 
 struct settings_list;
+struct filename_setting;
 void reset_setting(const struct settings_list *setting, void *var);
 void settings_reset(void);
 void sound_settings_apply(void);
@@ -267,6 +268,8 @@ const struct settings_list* find_setting_by_cfgname(const char* name, int *id);
 bool cfg_int_to_string(int setting_id, int val, char* buf, int buf_len);
 bool cfg_string_to_int(int setting_id, int* out, const char* str);
 bool cfg_to_string(int setting_id, char* buf, int buf_len);
+bool copy_filename_setting(char *buf, size_t buflen, const char *input,
+                           const struct filename_setting *fs);
 bool set_bool_options(const char* string, const bool* variable,
                       const char* yes_str, int yes_voice,
                       const char* no_str, int no_voice,