Fix potential buffer overflow with BLI_path_slash_ensure use

BLI_path_slash_ensure was appending to fixed sized buffers without
a size check.

13 files changed, 54 insertions, 50 deletions

diff --git a/source/blender/blenkernel/intern/appdir.c b/source/blender/blenkernel/intern/appdir.c
index 965f48d1e51..b3990b2b7fc 100644
--- a/source/blender/blenkernel/intern/appdir.c
+++ b/source/blender/blenkernel/intern/appdir.c
@@ -237,7 +237,7 @@ bool BKE_appdir_font_folder_default(char *dir)
   }
 #elif defined(__APPLE__)
   STRNCPY(test_dir, BLI_expand_tilde("~/Library/Fonts/"));
-  BLI_path_slash_ensure(test_dir);
+  BLI_path_slash_ensure(test_dir, sizeof(test_dir));
 #else
   STRNCPY(test_dir, "/usr/share/fonts");
 #endif
@@ -1093,13 +1093,13 @@ void BKE_appdir_app_templates(ListBase *templates)
  * \param userdir: Directory specified in user preferences (may be NULL).
  * note that by default this is an empty string, only use when non-empty.
  */
-static void where_is_temp(char *tempdir, const size_t tempdir_len, const char *userdir)
+static void where_is_temp(char *tempdir, const size_t tempdir_maxlen, const char *userdir)
 {
 
   tempdir[0] = '\0';
 
   if (userdir && BLI_is_dir(userdir)) {
-    BLI_strncpy(tempdir, userdir, tempdir_len);
+    BLI_strncpy(tempdir, userdir, tempdir_maxlen);
   }
 
   if (tempdir[0] == '\0') {
@@ -1116,23 +1116,23 @@ static void where_is_temp(char *tempdir, const size_t tempdir_len, const char *u
     for (int i = 0; i < ARRAY_SIZE(env_vars); i++) {
       const char *tmp = BLI_getenv(env_vars[i]);
       if (tmp && (tmp[0] != '\0') && BLI_is_dir(tmp)) {
-        BLI_strncpy(tempdir, tmp, tempdir_len);
+        BLI_strncpy(tempdir, tmp, tempdir_maxlen);
         break;
       }
     }
   }
 
   if (tempdir[0] == '\0') {
-    BLI_strncpy(tempdir, "/tmp/", tempdir_len);
+    BLI_strncpy(tempdir, "/tmp/", tempdir_maxlen);
   }
   else {
     /* add a trailing slash if needed */
-    BLI_path_slash_ensure(tempdir);
+    BLI_path_slash_ensure(tempdir, tempdir_maxlen);
   }
 }
 
 static void tempdir_session_create(char *tempdir_session,
-                                   const size_t tempdir_session_len,
+                                   const size_t tempdir_session_maxlen,
                                    const char *tempdir)
 {
   tempdir_session[0] = '\0';
@@ -1146,9 +1146,9 @@ static void tempdir_session_create(char *tempdir_session,
    * #_mktemp_s also requires the last null character is included. */
   const int tempdir_session_len_required = tempdir_len + session_name_len + 1;
 
-  if (tempdir_session_len_required <= tempdir_session_len) {
+  if (tempdir_session_len_required <= tempdir_session_maxlen) {
     /* No need to use path joining utility as we know the last character of #tempdir is a slash. */
-    BLI_string_join(tempdir_session, tempdir_session_len, tempdir, session_name);
+    BLI_string_join(tempdir_session, tempdir_session_maxlen, tempdir, session_name);
 #ifdef WIN32
     const bool needs_create = (_mktemp_s(tempdir_session, tempdir_session_len_required) == 0);
 #else
@@ -1158,7 +1158,7 @@ static void tempdir_session_create(char *tempdir_session,
       BLI_dir_create_recursive(tempdir_session);
     }
     if (BLI_is_dir(tempdir_session)) {
-      BLI_path_slash_ensure(tempdir_session);
+      BLI_path_slash_ensure(tempdir_session, tempdir_session_maxlen);
       /* Success. */
       return;
     }
@@ -1168,7 +1168,7 @@ static void tempdir_session_create(char *tempdir_session,
             "Could not generate a temp file name for '%s', falling back to '%s'",
             tempdir_session,
             tempdir);
-  BLI_strncpy(tempdir_session, tempdir, tempdir_session_len);
+  BLI_strncpy(tempdir_session, tempdir, tempdir_session_maxlen);
 }
 
 void BKE_tempdir_init(const char *userdir)
diff --git a/source/blender/blenkernel/intern/asset_library_service.cc b/source/blender/blenkernel/intern/asset_library_service.cc
index a6b2b7548a2..a4f97234042 100644
--- a/source/blender/blenkernel/intern/asset_library_service.cc
+++ b/source/blender/blenkernel/intern/asset_library_service.cc
@@ -44,7 +44,7 @@ std::string normalize_directory_path(StringRefNull directory)
 
   char dir_normalized[PATH_MAX];
   STRNCPY(dir_normalized, directory.c_str());
-  BLI_path_normalize_dir(nullptr, dir_normalized);
+  BLI_path_normalize_dir(nullptr, dir_normalized, sizeof(dir_normalized));
   return std::string(dir_normalized);
 }
 }  // namespace
diff --git a/source/blender/blenkernel/intern/asset_library_service_test.cc b/source/blender/blenkernel/intern/asset_library_service_test.cc
index d105c5644de..7952e7ea3b0 100644
--- a/source/blender/blenkernel/intern/asset_library_service_test.cc
+++ b/source/blender/blenkernel/intern/asset_library_service_test.cc
@@ -118,7 +118,7 @@ TEST_F(AssetLibraryServiceTest, library_path_trailing_slashes)
     asset_lib_no_slash[strlen(asset_lib_no_slash) - 1] = '\0';
   }
 
-  BLI_path_slash_ensure(asset_lib_with_slash);
+  BLI_path_slash_ensure(asset_lib_with_slash, PATH_MAX);
 
   AssetLibrary *const lib_no_slash = service->get_asset_library_on_disk(asset_lib_no_slash);
 
diff --git a/source/blender/blenkernel/intern/pointcache.c b/source/blender/blenkernel/intern/pointcache.c
index ac98bed2cf0..bb9a0f458b9 100644
--- a/source/blender/blenkernel/intern/pointcache.c
+++ b/source/blender/blenkernel/intern/pointcache.c
@@ -1317,7 +1317,7 @@ static int ptcache_path(PTCacheID *pid, char *dirname)
       BLI_path_abs(dirname, blendfilename);
     }
 
-    return BLI_path_slash_ensure(dirname); /* new strlen() */
+    return BLI_path_slash_ensure(dirname, MAX_PTCACHE_FILE); /* new strlen() */
   }
   if ((blendfile_path[0] != '\0') || lib) {
     char file[MAX_PTCACHE_PATH]; /* we don't want the dir, only the file */
@@ -1334,14 +1334,14 @@ static int ptcache_path(PTCacheID *pid, char *dirname)
     BLI_snprintf(dirname, MAX_PTCACHE_PATH, "//" PTCACHE_PATH "%s", file);
 
     BLI_path_abs(dirname, blendfilename);
-    return BLI_path_slash_ensure(dirname); /* new strlen() */
+    return BLI_path_slash_ensure(dirname, MAX_PTCACHE_FILE); /* new strlen() */
   }
 
   /* use the temp path. this is weak but better than not using point cache at all */
   /* temporary directory is assumed to exist and ALWAYS has a trailing slash */
   BLI_snprintf(dirname, MAX_PTCACHE_PATH, "%s" PTCACHE_PATH, BKE_tempdir_session());
 
-  return BLI_path_slash_ensure(dirname); /* new strlen() */
+  return BLI_path_slash_ensure(dirname, MAX_PTCACHE_FILE); /* new strlen() */
 }
 
 static size_t ptcache_filepath_ext_append(PTCacheID *pid,
diff --git a/source/blender/blenlib/BLI_path_util.h b/source/blender/blenlib/BLI_path_util.h
index d4d2ddead71..9c661178322 100644
--- a/source/blender/blenlib/BLI_path_util.h
+++ b/source/blender/blenlib/BLI_path_util.h
@@ -218,7 +218,7 @@ const char *BLI_path_slash_rfind(const char *string) ATTR_NONNULL() ATTR_WARN_UN
  * Appends a slash to string if there isn't one there already.
  * Returns the new length of the string.
  */
-int BLI_path_slash_ensure(char *string) ATTR_NONNULL();
+int BLI_path_slash_ensure(char *string, size_t string_maxlen) ATTR_NONNULL(1);
 /**
  * Removes the last slash and everything after it to the end of string, if there is one.
  */
@@ -314,7 +314,7 @@ void BLI_path_normalize(const char *relabase, char *path) ATTR_NONNULL(2);
  *
  * \note Same as #BLI_path_normalize but adds a trailing slash.
  */
-void BLI_path_normalize_dir(const char *relabase, char *dir) ATTR_NONNULL(2);
+void BLI_path_normalize_dir(const char *relabase, char *dir, size_t dir_maxlen) ATTR_NONNULL(2);
 
 /**
  * Make given name safe to be used in paths.
diff --git a/source/blender/blenlib/intern/fileops.c b/source/blender/blenlib/intern/fileops.c
index a157302e51e..005de1f85b4 100644
--- a/source/blender/blenlib/intern/fileops.c
+++ b/source/blender/blenlib/intern/fileops.c
@@ -401,7 +401,7 @@ static bool delete_recursive(const char *dir)
 
       /* dir listing produces dir path without trailing slash... */
       BLI_strncpy(path, fl->path, sizeof(path));
-      BLI_path_slash_ensure(path);
+      BLI_path_slash_ensure(path, sizeof(path));
 
       if (delete_recursive(path)) {
         err = true;
diff --git a/source/blender/blenlib/intern/path_util.c b/source/blender/blenlib/intern/path_util.c
index afe8c3cc033..759a1bf0dc7 100644
--- a/source/blender/blenlib/intern/path_util.c
+++ b/source/blender/blenlib/intern/path_util.c
@@ -218,7 +218,7 @@ void BLI_path_normalize(const char *relabase, char *path)
 #endif
 }
 
-void BLI_path_normalize_dir(const char *relabase, char *dir)
+void BLI_path_normalize_dir(const char *relabase, char *dir, size_t dir_maxlen)
 {
   /* Would just create an unexpected "/" path, just early exit entirely. */
   if (dir[0] == '\0') {
@@ -226,7 +226,7 @@ void BLI_path_normalize_dir(const char *relabase, char *dir)
   }
 
   BLI_path_normalize(relabase, dir);
-  BLI_path_slash_ensure(dir);
+  BLI_path_slash_ensure(dir, dir_maxlen);
 }
 
 bool BLI_filename_make_safe_ex(char *fname, bool allow_tokens)
@@ -1624,7 +1624,7 @@ bool BLI_path_contains(const char *container_path, const char *containee_path)
 
   /* Add a trailing slash to prevent same-prefix directories from matching.
    * e.g. "/some/path" doesn't contain "/some/path_lib". */
-  BLI_path_slash_ensure(container_native);
+  BLI_path_slash_ensure(container_native, sizeof(container_native));
 
   return BLI_str_startswith(containee_native, container_native);
 }
@@ -1659,13 +1659,17 @@ const char *BLI_path_slash_rfind(const char *string)
   return (lfslash > lbslash) ? lfslash : lbslash;
 }
 
-int BLI_path_slash_ensure(char *string)
+int BLI_path_slash_ensure(char *string, size_t string_maxlen)
 {
   int len = strlen(string);
+  BLI_assert(len < string_maxlen);
   if (len == 0 || string[len - 1] != SEP) {
-    string[len] = SEP;
-    string[len + 1] = '\0';
-    return len + 1;
+    /* Avoid unlikely buffer overflow. */
+    if (len + 1 < string_maxlen) {
+      string[len] = SEP;
+      string[len + 1] = '\0';
+      return len + 1;
+    }
   }
   return len;
 }
diff --git a/source/blender/editors/space_buttons/buttons_ops.c b/source/blender/editors/space_buttons/buttons_ops.c
index a9ce9a3d723..4013288b13b 100644
--- a/source/blender/editors/space_buttons/buttons_ops.c
+++ b/source/blender/editors/space_buttons/buttons_ops.c
@@ -205,7 +205,7 @@ static int file_browse_exec(bContext *C, wmOperator *op)
 
     if (BLI_is_dir(path)) {
       /* Do this first so '//' isn't converted to '//\' on windows. */
-      BLI_path_slash_ensure(path);
+      BLI_path_slash_ensure(path, sizeof(path));
       if (is_relative) {
         BLI_path_rel(path, BKE_main_blendfile_path(bmain));
         str_len = strlen(path);
diff --git a/source/blender/editors/space_file/file_ops.c b/source/blender/editors/space_file/file_ops.c
index 20025b0bac9..c9c4704d16c 100644
--- a/source/blender/editors/space_file/file_ops.c
+++ b/source/blender/editors/space_file/file_ops.c
@@ -197,13 +197,13 @@ static FileSelect file_select_do(bContext *C, int selected_idx, bool do_diropen)
         }
         else if (file->redirection_path) {
           BLI_strncpy(params->dir, file->redirection_path, sizeof(params->dir));
-          BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), params->dir);
-          BLI_path_slash_ensure(params->dir);
+          BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), params->dir, sizeof(params->dir));
+          BLI_path_slash_ensure(params->dir, sizeof(params->dir));
         }
         else {
-          BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), params->dir);
+          BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), params->dir, sizeof(params->dir));
           strcat(params->dir, file->relpath);
-          BLI_path_slash_ensure(params->dir);
+          BLI_path_slash_ensure(params->dir, sizeof(params->dir));
         }
 
         ED_file_change_dir(C);
@@ -1095,7 +1095,7 @@ static int bookmark_select_exec(bContext *C, wmOperator *op)
 
   RNA_property_string_get(op->ptr, prop, entry);
   BLI_strncpy(params->dir, entry, sizeof(params->dir));
-  BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), params->dir);
+  BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), params->dir, sizeof(params->dir));
   ED_file_change_dir(C);
 
   WM_event_add_notifier(C, NC_SPACE | ND_SPACE_FILE_LIST, NULL);
@@ -1568,8 +1568,8 @@ void file_sfile_to_operator_ex(
     BLI_path_join(filepath, FILE_MAX, params->dir, params->file);
   }
   else {
-    BLI_strncpy(filepath, params->dir, FILE_MAX - 1);
-    BLI_path_slash_ensure(filepath);
+    BLI_strncpy(filepath, params->dir, FILE_MAX);
+    BLI_path_slash_ensure(filepath, FILE_MAX);
   }
 
   if ((prop = RNA_struct_find_property(op->ptr, "relative_path"))) {
@@ -1797,8 +1797,8 @@ static bool file_execute(bContext *C, SpaceFile *sfile)
     }
     else {
       BLI_path_normalize(BKE_main_blendfile_path(bmain), params->dir);
-      BLI_path_append(params->dir, sizeof(params->dir) - 1, file->relpath);
-      BLI_path_slash_ensure(params->dir);
+      BLI_path_append(params->dir, sizeof(params->dir), file->relpath);
+      BLI_path_slash_ensure(params->dir, sizeof(params->dir));
     }
     ED_file_change_dir(C);
   }
@@ -1961,7 +1961,7 @@ static int file_parent_exec(bContext *C, wmOperator *UNUSED(unused))
 
   if (params) {
     if (BLI_path_parent_dir(params->dir)) {
-      BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), params->dir);
+      BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), params->dir, sizeof(params->dir));
       ED_file_change_dir(C);
       if (params->recursion_level > 1) {
         /* Disable 'dirtree' recursion when going up in tree. */
@@ -2544,7 +2544,7 @@ void file_directory_enter_handle(bContext *C, void *UNUSED(arg_unused), void *UN
       }
     }
 
-    BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), params->dir);
+    BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), params->dir, sizeof(params->dir));
 
     if (filelist_is_dir(sfile->files, params->dir)) {
       if (!STREQ(params->dir, old_dir)) { /* Avoids flickering when nothing's changed. */
@@ -2631,7 +2631,7 @@ void file_filename_enter_handle(bContext *C, void *UNUSED(arg_unused), void *arg
 
       /* if directory, open it and empty filename field */
       if (filelist_is_dir(sfile->files, filepath)) {
-        BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), filepath);
+        BLI_path_normalize_dir(BKE_main_blendfile_path(bmain), filepath, sizeof(filepath));
         BLI_strncpy(params->dir, filepath, sizeof(params->dir));
         params->file[0] = '\0';
         ED_file_change_dir(C);
diff --git a/source/blender/editors/space_file/filelist.cc b/source/blender/editors/space_file/filelist.cc
index f177eebf6f2..3257534f94d 100644
--- a/source/blender/editors/space_file/filelist.cc
+++ b/source/blender/editors/space_file/filelist.cc
@@ -1208,7 +1208,7 @@ static int filelist_geticon_ex(const FileDirEntry *file,
       }
       else if (root) {
         BLI_path_join(fullpath, sizeof(fullpath), root, file->relpath);
-        BLI_path_slash_ensure(fullpath);
+        BLI_path_slash_ensure(fullpath, sizeof(fullpath));
       }
       for (; tfsm; tfsm = tfsm->next) {
         if (STREQ(tfsm->path, target)) {
@@ -1952,7 +1952,7 @@ void filelist_setdir(struct FileList *filelist, char *r_dir)
   const bool allow_invalid = filelist->asset_library_ref != nullptr;
   BLI_assert(strlen(r_dir) < FILE_MAX_LIBEXTRA);
 
-  BLI_path_normalize_dir(BKE_main_blendfile_path_from_global(), r_dir);
+  BLI_path_normalize_dir(BKE_main_blendfile_path_from_global(), r_dir, FILE_MAX_LIBEXTRA);
   const bool is_valid_path = filelist->check_dir_fn(filelist, r_dir, !allow_invalid);
   BLI_assert(is_valid_path || allow_invalid);
   UNUSED_VARS_NDEBUG(is_valid_path);
@@ -2920,7 +2920,7 @@ static int filelist_readjob_list_dir(const char *root,
         if (BLI_file_alias_target(full_path, entry->redirection_path)) {
           if (BLI_is_dir(entry->redirection_path)) {
             entry->typeflag = FILE_TYPE_DIR;
-            BLI_path_slash_ensure(entry->redirection_path);
+            BLI_path_slash_ensure(entry->redirection_path, FILE_MAXDIR);
           }
           else {
             entry->typeflag = (eFileSel_File_Types)ED_path_extension_type(entry->redirection_path);
@@ -3476,7 +3476,7 @@ static void filelist_readjob_recursive_dir_add_items(const bool do_lib,
   BLI_strncpy(dir, filelist->filelist.root, sizeof(dir));
   BLI_strncpy(filter_glob, filelist->filter_data.filter_glob, sizeof(filter_glob));
 
-  BLI_path_normalize_dir(job_params->main_name, dir);
+  BLI_path_normalize_dir(job_params->main_name, dir, sizeof(dir));
   td_dir->dir = BLI_strdup(dir);
 
   /* Init the file indexer. */
@@ -3507,7 +3507,7 @@ static void filelist_readjob_recursive_dir_add_items(const bool do_lib,
      * Note that in the end, this means we 'cache' valid relative subdir once here,
      * this is actually better. */
     BLI_strncpy(rel_subdir, subdir, sizeof(rel_subdir));
-    BLI_path_normalize_dir(root, rel_subdir);
+    BLI_path_normalize_dir(root, rel_subdir, sizeof(rel_subdir));
     BLI_path_rel(rel_subdir, root);
 
     bool is_lib = false;
@@ -3555,7 +3555,7 @@ static void filelist_readjob_recursive_dir_add_items(const bool do_lib,
               max_recursion, is_lib, recursion_level, entry)) {
         /* We have a directory we want to list, add it to todo list! */
         BLI_path_join(dir, sizeof(dir), root, entry->relpath);
-        BLI_path_normalize_dir(job_params->main_name, dir);
+        BLI_path_normalize_dir(job_params->main_name, dir, sizeof(dir));
         td_dir = static_cast<TodoDir *>(BLI_stack_push_r(todo_dirs));
         td_dir->level = recursion_level + 1;
         td_dir->dir = BLI_strdup(dir);
diff --git a/source/blender/editors/space_file/filesel.c b/source/blender/editors/space_file/filesel.c
index df7e9702e85..af2c9d4e757 100644
--- a/source/blender/editors/space_file/filesel.c
+++ b/source/blender/editors/space_file/filesel.c
@@ -197,7 +197,7 @@ static FileSelectParams *fileselect_ensure_updated_file_params(SpaceFile *sfile)
     }
 
     if (params->dir[0]) {
-      BLI_path_normalize_dir(blendfile_path, params->dir);
+      BLI_path_normalize_dir(blendfile_path, params->dir, sizeof(params->dir));
       BLI_path_abs(params->dir, blendfile_path);
     }
 
@@ -1187,7 +1187,7 @@ int autocomplete_directory(struct bContext *C, char *str, void *UNUSED(arg_v))
 
       match = UI_autocomplete_end(autocpl, str);
       if (match == AUTOCOMPLETE_FULL_MATCH) {
-        BLI_path_slash_ensure(str);
+        BLI_path_slash_ensure(str, FILE_MAX);
       }
     }
   }
diff --git a/source/blender/makesrna/intern/rna_userdef.c b/source/blender/makesrna/intern/rna_userdef.c
index e22ae205b0a..58189ab2478 100644
--- a/source/blender/makesrna/intern/rna_userdef.c
+++ b/source/blender/makesrna/intern/rna_userdef.c
@@ -574,7 +574,7 @@ static void rna_Userdef_disk_cache_dir_update(Main *UNUSED(bmain),
 {
   if (U.sequencer_disk_cache_dir[0] != '\0') {
     BLI_path_abs(U.sequencer_disk_cache_dir, BKE_main_blendfile_path_from_global());
-    BLI_path_slash_ensure(U.sequencer_disk_cache_dir);
+    BLI_path_slash_ensure(U.sequencer_disk_cache_dir, sizeof(U.sequencer_disk_cache_dir));
     BLI_path_make_safe(U.sequencer_disk_cache_dir);
   }
 
diff --git a/source/blender/sequencer/intern/disk_cache.c b/source/blender/sequencer/intern/disk_cache.c
index 1f52d2ea41b..8c0ae4e055c 100644
--- a/source/blender/sequencer/intern/disk_cache.c
+++ b/source/blender/sequencer/intern/disk_cache.c
@@ -182,7 +182,7 @@ static void seq_disk_cache_get_files(SeqDiskCache *disk_cache, char *path)
     if (is_dir && !FILENAME_IS_CURRPAR(file)) {
       char subpath[FILE_MAX];
       BLI_strncpy(subpath, fl->path, sizeof(subpath));
-      BLI_path_slash_ensure(subpath);
+      BLI_path_slash_ensure(subpath, sizeof(sizeof(subpath)));
       seq_disk_cache_get_files(disk_cache, subpath);
     }
 
@@ -384,7 +384,7 @@ static void seq_disk_cache_delete_invalid_files(SeqDiskCache *disk_cache,
   DiskCacheFile *next_file, *cache_file = disk_cache->files.first;
   char cache_dir[FILE_MAX];
   seq_disk_cache_get_dir(disk_cache, scene, seq, cache_dir, sizeof(cache_dir));
-  BLI_path_slash_ensure(cache_dir);
+  BLI_path_slash_ensure(cache_dir, sizeof(cache_dir));
 
   while (cache_file) {
     next_file = cache_file->next;