[Bf-blender-cvs] [d66f24cfe30] master: Fix potential buffer overflow with BLI_path_slash_ensure use
Campbell Barton
noreply at git.blender.org
Sun Oct 30 06:09:35 CET 2022
Commit: d66f24cfe30d26e03863a78de9fd58bb3b65ed43
Author: Campbell Barton
Date: Sun Oct 30 15:34:02 2022 +1100
Branches: master
https://developer.blender.org/rBd66f24cfe30d26e03863a78de9fd58bb3b65ed43
Fix potential buffer overflow with BLI_path_slash_ensure use
BLI_path_slash_ensure was appending to fixed sized buffers without
a size check.
===================================================================
M source/blender/blenkernel/intern/appdir.c
M source/blender/blenkernel/intern/asset_library_service.cc
M source/blender/blenkernel/intern/asset_library_service_test.cc
M source/blender/blenkernel/intern/pointcache.c
M source/blender/blenlib/BLI_path_util.h
M source/blender/blenlib/intern/fileops.c
M source/blender/blenlib/intern/path_util.c
M source/blender/editors/space_buttons/buttons_ops.c
M source/blender/editors/space_file/file_ops.c
M source/blender/editors/space_file/filelist.cc
M source/blender/editors/space_file/filesel.c
M source/blender/makesrna/intern/rna_userdef.c
M source/blender/sequencer/intern/disk_cache.c
===================================================================
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,
@@ Diff output truncated at 10240 characters. @@
More information about the Bf-blender-cvs
mailing list