Fix buffer overflow in BKE_bpath_foreach_path_fixed_process

It was assumed destination buffers were at least 1024 bytes which could
overflow by 256 bytes for sequencer directories. Resolve by passing the
destination buffer size to BKE_bpath_foreach_path_fixed_process.

Also remove strcpy use in foreach_path_clean_cb.
This commit is contained in:
Campbell Barton
2023-06-23 10:09:01 +10:00
parent 06cb4ca376
commit 1a04a231ec
19 changed files with 111 additions and 65 deletions

View File

@@ -28,11 +28,12 @@ struct ReportList;
* \{ */
typedef enum eBPathForeachFlag {
/** Flags controlling the behavior of the generic BPath API. */
/** Ensures the `absolute_base_path` member of #BPathForeachPathData is initialized properly with
/* Flags controlling the behavior of the generic BPath API. */
/**
* Ensures the `absolute_base_path` member of #BPathForeachPathData is initialized properly with
* the path of the current .blend file. This can be used by the callbacks to convert relative
* paths to absolute ones. */
* paths to absolute ones.
*/
BKE_BPATH_FOREACH_PATH_ABSOLUTE = (1 << 0),
/** Skip paths of linked IDs. */
BKE_BPATH_FOREACH_PATH_SKIP_LINKED = (1 << 1),
@@ -68,13 +69,19 @@ struct BPathForeachPathData;
/**
* Callback used to iterate over an ID's file paths.
* \param path_dst: Optionally write to the path (for callbacks that manipulate the path).
* \note When #BKE_BPATH_FOREACH_PATH_ABSOLUTE us used, `path_src` will be absolute and `path_dst`
* can be used to access the original path.
* \param path_dst_maxncpy: The buffer size of `path_dst` including the null byte.
* \warning Actions such as #BLI_path_abs & #BLI_path_rel must not be called directly
* on `path_dst` as they assume #FILE_MAX size which may not be the case.
*
* \note `path`s parameters should be considered as having a maximal `FILE_MAX` string length.
*
* \return `true` if the path has been changed, and in that case, result should be written into
* `r_path_dst`. */
* \return `true` if the path has been changed, and in that case,
* result must be written to `path_dst`.
*/
typedef bool (*BPathForeachPathFunctionCallback)(struct BPathForeachPathData *bpath_data,
char *r_path_dst,
char *path_dst,
size_t path_dst_maxncpy,
const char *path_src);
/** Storage for common data needed across the BPath 'foreach_path' code. */
@@ -122,7 +129,9 @@ void BKE_bpath_foreach_path_main(BPathForeachPathData *bpath_data);
*
* \return true is \a path was modified, false otherwise.
*/
bool BKE_bpath_foreach_path_fixed_process(struct BPathForeachPathData *bpath_data, char *path);
bool BKE_bpath_foreach_path_fixed_process(struct BPathForeachPathData *bpath_data,
char *path,
size_t path_maxncpy);
/**
* Run the callback on a (directory + file) path, replacing the content of the two strings as
@@ -135,7 +144,9 @@ bool BKE_bpath_foreach_path_fixed_process(struct BPathForeachPathData *bpath_dat
*/
bool BKE_bpath_foreach_path_dirfile_fixed_process(struct BPathForeachPathData *bpath_data,
char *path_dir,
char *path_file);
size_t path_dir_maxncpy,
char *path_file,
size_t path_file_maxncpy);
/**
* Run the callback on a path, replacing the content of the string as needed.

View File

@@ -167,9 +167,10 @@ static bool blendfile_or_libraries_versions_atleast(Main *bmain,
static bool foreach_path_clean_cb(BPathForeachPathData * /*bpath_data*/,
char *path_dst,
size_t path_dst_maxncpy,
const char *path_src)
{
strcpy(path_dst, path_src);
BLI_strncpy(path_dst, path_src, path_dst_maxncpy);
BLI_path_slash_native(path_dst);
return !STREQ(path_dst, path_src);
}

View File

@@ -97,7 +97,9 @@ void BKE_bpath_foreach_path_id(BPathForeachPathData *bpath_data, ID *id)
if (id->library_weak_reference != NULL && (flag & BKE_BPATH_TRAVERSE_SKIP_WEAK_REFERENCES) == 0)
{
BKE_bpath_foreach_path_fixed_process(bpath_data, id->library_weak_reference->library_filepath);
BKE_bpath_foreach_path_fixed_process(bpath_data,
id->library_weak_reference->library_filepath,
sizeof(id->library_weak_reference->library_filepath));
}
bNodeTree *embedded_node_tree = ntreeFromID(id);
@@ -128,7 +130,9 @@ void BKE_bpath_foreach_path_main(BPathForeachPathData *bpath_data)
FOREACH_MAIN_ID_END;
}
bool BKE_bpath_foreach_path_fixed_process(BPathForeachPathData *bpath_data, char *path)
bool BKE_bpath_foreach_path_fixed_process(BPathForeachPathData *bpath_data,
char *path,
size_t path_maxncpy)
{
const char *absolute_base_path = bpath_data->absolute_base_path;
@@ -148,8 +152,8 @@ bool BKE_bpath_foreach_path_fixed_process(BPathForeachPathData *bpath_data, char
/* so functions can check old value */
STRNCPY(path_dst, path);
if (bpath_data->callback_function(bpath_data, path_dst, path_src)) {
BLI_strncpy(path, path_dst, FILE_MAX);
if (bpath_data->callback_function(bpath_data, path_dst, sizeof(path_dst), path_src)) {
BLI_strncpy(path, path_dst, path_maxncpy);
bpath_data->is_path_modified = true;
return true;
}
@@ -159,7 +163,9 @@ bool BKE_bpath_foreach_path_fixed_process(BPathForeachPathData *bpath_data, char
bool BKE_bpath_foreach_path_dirfile_fixed_process(BPathForeachPathData *bpath_data,
char *path_dir,
char *path_file)
size_t path_dir_maxncpy,
char *path_file,
size_t path_file_maxncpy)
{
const char *absolute_base_path = bpath_data->absolute_base_path;
@@ -175,8 +181,9 @@ bool BKE_bpath_foreach_path_dirfile_fixed_process(BPathForeachPathData *bpath_da
BLI_path_abs(path_src, absolute_base_path);
}
if (bpath_data->callback_function(bpath_data, path_dst, (const char *)path_src)) {
BLI_path_split_dir_file(path_dst, path_dir, FILE_MAXDIR, path_file, FILE_MAXFILE);
if (bpath_data->callback_function(
bpath_data, path_dst, sizeof(path_dst), (const char *)path_src)) {
BLI_path_split_dir_file(path_dst, path_dir, path_dir_maxncpy, path_file, path_file_maxncpy);
bpath_data->is_path_modified = true;
return true;
}
@@ -201,7 +208,7 @@ bool BKE_bpath_foreach_path_allocated_process(BPathForeachPathData *bpath_data,
path_src = *path;
}
if (bpath_data->callback_function(bpath_data, path_dst, path_src)) {
if (bpath_data->callback_function(bpath_data, path_dst, sizeof(path_dst), path_src)) {
MEM_freeN(*path);
(*path) = BLI_strdup(path_dst);
bpath_data->is_path_modified = true;
@@ -219,6 +226,8 @@ bool BKE_bpath_foreach_path_allocated_process(BPathForeachPathData *bpath_data,
static bool check_missing_files_foreach_path_cb(BPathForeachPathData *bpath_data,
char *UNUSED(path_dst),
size_t UNUSED(path_dst_maxncpy),
const char *path_src)
{
ReportList *reports = (ReportList *)bpath_data->user_data;
@@ -337,6 +346,7 @@ typedef struct BPathFind_Data {
static bool missing_files_find_foreach_path_cb(BPathForeachPathData *bpath_data,
char *path_dst,
size_t path_dst_maxncpy,
const char *path_src)
{
BPathFind_Data *data = (BPathFind_Data *)bpath_data->user_data;
@@ -371,15 +381,11 @@ static bool missing_files_find_foreach_path_cb(BPathForeachPathData *bpath_data,
return false;
}
bool was_relative = BLI_path_is_rel(path_dst);
BLI_strncpy(path_dst, filepath_new, FILE_MAX);
/* Keep the path relative if the previous one was relative. */
if (was_relative) {
BLI_path_rel(path_dst, data->basedir);
if (BLI_path_is_rel(path_dst)) {
BLI_path_rel(filepath_new, data->basedir);
}
BLI_strncpy(path_dst, filepath_new, path_dst_maxncpy);
return true;
}
@@ -425,6 +431,7 @@ typedef struct BPathRebase_Data {
static bool relative_rebase_foreach_path_cb(BPathForeachPathData *bpath_data,
char *path_dst,
size_t path_dst_maxncpy,
const char *path_src)
{
BPathRebase_Data *data = (BPathRebase_Data *)bpath_data->user_data;
@@ -449,7 +456,7 @@ static bool relative_rebase_foreach_path_cb(BPathForeachPathData *bpath_data,
/* This may fail, if so it's fine to leave absolute since the path is still valid. */
BLI_path_rel(filepath, data->basedir_dst);
BLI_strncpy(path_dst, filepath, FILE_MAX);
BLI_strncpy(path_dst, filepath, path_dst_maxncpy);
data->count_changed++;
return true;
}
@@ -500,6 +507,7 @@ typedef struct BPathRemap_Data {
static bool relative_convert_foreach_path_cb(BPathForeachPathData *bpath_data,
char *path_dst,
size_t path_dst_maxncpy,
const char *path_src)
{
BPathRemap_Data *data = (BPathRemap_Data *)bpath_data->user_data;
@@ -510,12 +518,12 @@ static bool relative_convert_foreach_path_cb(BPathForeachPathData *bpath_data,
return false; /* Already relative. */
}
BLI_strncpy(path_dst, path_src, FILE_MAX);
BLI_path_rel(path_dst, data->basedir);
if (BLI_path_is_rel(path_dst)) {
data->count_changed++;
}
else {
char path_test[FILE_MAX];
STRNCPY(path_test, path_src);
BLI_path_rel(path_test, data->basedir);
BLI_strncpy(path_dst, path_test, path_dst_maxncpy);
if (!BLI_path_is_rel(path_test)) {
const char *type_name = BKE_idtype_get_info_from_id(bpath_data->owner_id)->name;
const char *id_name = bpath_data->owner_id->name + 2;
BKE_reportf(data->reports,
@@ -525,12 +533,16 @@ static bool relative_convert_foreach_path_cb(BPathForeachPathData *bpath_data,
type_name,
id_name);
data->count_failed++;
return true;
}
data->count_changed++;
return true;
}
static bool absolute_convert_foreach_path_cb(BPathForeachPathData *bpath_data,
char *path_dst,
size_t path_dst_maxncpy,
const char *path_src)
{
BPathRemap_Data *data = (BPathRemap_Data *)bpath_data->user_data;
@@ -541,12 +553,11 @@ static bool absolute_convert_foreach_path_cb(BPathForeachPathData *bpath_data,
return false; /* Already absolute. */
}
BLI_strncpy(path_dst, path_src, FILE_MAX);
BLI_path_abs(path_dst, data->basedir);
if (BLI_path_is_rel(path_dst) == false) {
data->count_changed++;
}
else {
char path_test[FILE_MAX];
STRNCPY(path_test, path_src);
BLI_path_abs(path_test, data->basedir);
BLI_strncpy(path_dst, path_test, path_dst_maxncpy);
if (BLI_path_is_rel(path_test)) {
const char *type_name = BKE_idtype_get_info_from_id(bpath_data->owner_id)->name;
const char *id_name = bpath_data->owner_id->name + 2;
BKE_reportf(data->reports,
@@ -556,7 +567,10 @@ static bool absolute_convert_foreach_path_cb(BPathForeachPathData *bpath_data,
type_name,
id_name);
data->count_failed++;
return true;
}
data->count_changed++;
return true;
}
@@ -611,6 +625,7 @@ struct PathStore {
static bool bpath_list_append(BPathForeachPathData *bpath_data,
char *UNUSED(path_dst),
size_t UNUSED(path_dst_maxncpy),
const char *path_src)
{
ListBase *path_list = bpath_data->user_data;
@@ -627,6 +642,7 @@ static bool bpath_list_append(BPathForeachPathData *bpath_data,
static bool bpath_list_restore(BPathForeachPathData *bpath_data,
char *path_dst,
size_t path_dst_maxncpy,
const char *path_src)
{
ListBase *path_list = bpath_data->user_data;
@@ -640,7 +656,7 @@ static bool bpath_list_restore(BPathForeachPathData *bpath_data,
bool is_path_changed = false;
if (!STREQ(path_src, filepath)) {
BLI_strncpy(path_dst, filepath, FILE_MAX);
BLI_strncpy(path_dst, filepath, path_dst_maxncpy);
is_path_changed = true;
}

View File

@@ -208,7 +208,8 @@ static void brush_foreach_path(ID *id, BPathForeachPathData *bpath_data)
{
Brush *brush = (Brush *)id;
if (brush->icon_filepath[0] != '\0') {
BKE_bpath_foreach_path_fixed_process(bpath_data, brush->icon_filepath);
BKE_bpath_foreach_path_fixed_process(
bpath_data, brush->icon_filepath, sizeof(brush->icon_filepath));
}
}

View File

@@ -87,7 +87,8 @@ static void cache_file_free_data(ID *id)
static void cache_file_foreach_path(ID *id, BPathForeachPathData *bpath_data)
{
CacheFile *cache_file = (CacheFile *)id;
BKE_bpath_foreach_path_fixed_process(bpath_data, cache_file->filepath);
BKE_bpath_foreach_path_fixed_process(
bpath_data, cache_file->filepath, sizeof(cache_file->filepath));
}
static void cache_file_blend_write(BlendWriter *writer, ID *id, const void *id_address)

View File

@@ -300,7 +300,7 @@ static void image_foreach_path(ID *id, BPathForeachPathData *bpath_data)
temp_path, udim_pattern, tile_format, ((ImageTile *)ima->tiles.first)->tile_number);
MEM_SAFE_FREE(udim_pattern);
result = BKE_bpath_foreach_path_fixed_process(bpath_data, temp_path);
result = BKE_bpath_foreach_path_fixed_process(bpath_data, temp_path, sizeof(temp_path));
if (result) {
/* Put the filepath back together using the new directory and the original file name. */
char new_dir[FILE_MAXDIR];
@@ -309,7 +309,8 @@ static void image_foreach_path(ID *id, BPathForeachPathData *bpath_data)
}
}
else {
result = BKE_bpath_foreach_path_fixed_process(bpath_data, ima->filepath);
result = BKE_bpath_foreach_path_fixed_process(
bpath_data, ima->filepath, sizeof(ima->filepath));
}
if (result) {

View File

@@ -119,7 +119,8 @@ IDTypeInfo IDType_ID_LINK_PLACEHOLDER = {
* absolute, in which case it is not altered.
*/
static bool lib_id_library_local_paths_callback(BPathForeachPathData *bpath_data,
char *r_path_dst,
char *path_dst,
size_t path_dst_maxncpy,
const char *path_src)
{
const char **data = bpath_data->user_data;
@@ -142,7 +143,7 @@ static bool lib_id_library_local_paths_callback(BPathForeachPathData *bpath_data
* because it won't work for paths that start with "//../" */
BLI_path_normalize(filepath);
BLI_path_rel(filepath, base_new);
BLI_strncpy(r_path_dst, filepath, FILE_MAX);
BLI_strncpy(path_dst, filepath, path_dst_maxncpy);
return true;
}

View File

@@ -68,7 +68,7 @@ static void library_foreach_path(ID *id, BPathForeachPathData *bpath_data)
return;
}
if (BKE_bpath_foreach_path_fixed_process(bpath_data, lib->filepath)) {
if (BKE_bpath_foreach_path_fixed_process(bpath_data, lib->filepath, sizeof(lib->filepath))) {
BKE_library_filepath_set(bpath_data->bmain, lib, lib->filepath);
}
}

View File

@@ -221,7 +221,8 @@ static void mesh_foreach_path(ID *id, BPathForeachPathData *bpath_data)
{
Mesh *me = (Mesh *)id;
if (me->ldata.external) {
BKE_bpath_foreach_path_fixed_process(bpath_data, me->ldata.external->filepath);
BKE_bpath_foreach_path_fixed_process(
bpath_data, me->ldata.external->filepath, sizeof(me->ldata.external->filepath));
}
}

View File

@@ -145,7 +145,8 @@ static void movie_clip_foreach_cache(ID *id,
static void movie_clip_foreach_path(ID *id, BPathForeachPathData *bpath_data)
{
MovieClip *movie_clip = (MovieClip *)id;
BKE_bpath_foreach_path_fixed_process(bpath_data, movie_clip->filepath);
BKE_bpath_foreach_path_fixed_process(
bpath_data, movie_clip->filepath, sizeof(movie_clip->filepath));
}
static void write_movieTracks(BlendWriter *writer, ListBase *tracks)

View File

@@ -436,11 +436,11 @@ static void node_foreach_path(ID *id, BPathForeachPathData *bpath_data)
for (bNode *node : ntree->all_nodes()) {
if (node->type == SH_NODE_SCRIPT) {
NodeShaderScript *nss = static_cast<NodeShaderScript *>(node->storage);
BKE_bpath_foreach_path_fixed_process(bpath_data, nss->filepath);
BKE_bpath_foreach_path_fixed_process(bpath_data, nss->filepath, sizeof(nss->filepath));
}
else if (node->type == SH_NODE_TEX_IES) {
NodeShaderTexIES *ies = static_cast<NodeShaderTexIES *>(node->storage);
BKE_bpath_foreach_path_fixed_process(bpath_data, ies->filepath);
BKE_bpath_foreach_path_fixed_process(bpath_data, ies->filepath, sizeof(ies->filepath));
}
}
break;

View File

@@ -489,7 +489,7 @@ static void object_foreach_path_pointcache(ListBase *ptcache_list,
for (PointCache *cache = (PointCache *)ptcache_list->first; cache != nullptr;
cache = cache->next) {
if (cache->flag & PTCACHE_DISK_CACHE) {
BKE_bpath_foreach_path_fixed_process(bpath_data, cache->path);
BKE_bpath_foreach_path_fixed_process(bpath_data, cache->path, sizeof(cache->path));
}
}
}
@@ -504,14 +504,16 @@ static void object_foreach_path(ID *id, BPathForeachPathData *bpath_data)
case eModifierType_Fluidsim: {
FluidsimModifierData *fluidmd = reinterpret_cast<FluidsimModifierData *>(md);
if (fluidmd->fss) {
BKE_bpath_foreach_path_fixed_process(bpath_data, fluidmd->fss->surfdataPath);
BKE_bpath_foreach_path_fixed_process(
bpath_data, fluidmd->fss->surfdataPath, sizeof(fluidmd->fss->surfdataPath));
}
break;
}
case eModifierType_Fluid: {
FluidModifierData *fmd = reinterpret_cast<FluidModifierData *>(md);
if (fmd->type & MOD_FLUID_TYPE_DOMAIN && fmd->domain) {
BKE_bpath_foreach_path_fixed_process(bpath_data, fmd->domain->cache_directory);
BKE_bpath_foreach_path_fixed_process(
bpath_data, fmd->domain->cache_directory, sizeof(fmd->domain->cache_directory));
}
break;
}
@@ -522,12 +524,12 @@ static void object_foreach_path(ID *id, BPathForeachPathData *bpath_data)
}
case eModifierType_Ocean: {
OceanModifierData *omd = reinterpret_cast<OceanModifierData *>(md);
BKE_bpath_foreach_path_fixed_process(bpath_data, omd->cachepath);
BKE_bpath_foreach_path_fixed_process(bpath_data, omd->cachepath, sizeof(omd->cachepath));
break;
}
case eModifierType_MeshCache: {
MeshCacheModifierData *mcmd = reinterpret_cast<MeshCacheModifierData *>(md);
BKE_bpath_foreach_path_fixed_process(bpath_data, mcmd->filepath);
BKE_bpath_foreach_path_fixed_process(bpath_data, mcmd->filepath, sizeof(mcmd->filepath));
break;
}
default:

View File

@@ -945,7 +945,11 @@ static bool seq_foreach_path_callback(Sequence *seq, void *user_data)
BPathForeachPathData *bpath_data = (BPathForeachPathData *)user_data;
if (ELEM(seq->type, SEQ_TYPE_MOVIE, SEQ_TYPE_SOUND_RAM) && se) {
BKE_bpath_foreach_path_dirfile_fixed_process(bpath_data, seq->strip->dirpath, se->filename);
BKE_bpath_foreach_path_dirfile_fixed_process(bpath_data,
seq->strip->dirpath,
sizeof(seq->strip->dirpath),
se->filename,
sizeof(se->filename));
}
else if ((seq->type == SEQ_TYPE_IMAGE) && se) {
/* NOTE: An option not to loop over all strips could be useful? */
@@ -958,13 +962,17 @@ static bool seq_foreach_path_callback(Sequence *seq, void *user_data)
}
for (i = 0; i < len; i++, se++) {
BKE_bpath_foreach_path_dirfile_fixed_process(
bpath_data, seq->strip->dirpath, se->filename);
BKE_bpath_foreach_path_dirfile_fixed_process(bpath_data,
seq->strip->dirpath,
sizeof(seq->strip->dirpath),
se->filename,
sizeof(se->filename));
}
}
else {
/* simple case */
BKE_bpath_foreach_path_fixed_process(bpath_data, seq->strip->dirpath);
BKE_bpath_foreach_path_fixed_process(
bpath_data, seq->strip->dirpath, sizeof(seq->strip->dirpath));
}
}
return true;

View File

@@ -127,7 +127,7 @@ static void sound_foreach_path(ID *id, BPathForeachPathData *bpath_data)
}
/* FIXME: This does not check for empty path... */
BKE_bpath_foreach_path_fixed_process(bpath_data, sound->filepath);
BKE_bpath_foreach_path_fixed_process(bpath_data, sound->filepath, sizeof(sound->filepath));
}
static void sound_blend_write(BlendWriter *writer, ID *id, const void *id_address)

View File

@@ -121,7 +121,7 @@ static void vfont_foreach_path(ID *id, BPathForeachPathData *bpath_data)
return;
}
BKE_bpath_foreach_path_fixed_process(bpath_data, vfont->filepath);
BKE_bpath_foreach_path_fixed_process(bpath_data, vfont->filepath, sizeof(vfont->filepath));
}
static void vfont_blend_write(BlendWriter *writer, ID *id, const void *id_address)

View File

@@ -585,7 +585,7 @@ static void volume_foreach_path(ID *id, BPathForeachPathData *bpath_data)
return;
}
BKE_bpath_foreach_path_fixed_process(bpath_data, volume->filepath);
BKE_bpath_foreach_path_fixed_process(bpath_data, volume->filepath, sizeof(volume->filepath));
}
static void volume_blend_write(BlendWriter *writer, ID *id, const void *id_address)

View File

@@ -889,6 +889,7 @@ struct FileCheckCallbackInfo {
static bool external_file_check_callback(BPathForeachPathData *bpath_data,
char * /*path_dst*/,
size_t /*path_dst_maxncpy*/,
const char *path_src)
{
FileCheckCallbackInfo *callback_info = static_cast<FileCheckCallbackInfo *>(

View File

@@ -104,4 +104,4 @@ void ED_operatortypes_grease_pencil_layers(void)
using namespace blender::ed::greasepencil;
WM_operatortype_append(GREASE_PENCIL_OT_layer_add);
WM_operatortype_append(GREASE_PENCIL_OT_layer_remove);
}
}

View File

@@ -89,6 +89,7 @@ static PyObject *bpy_script_paths(PyObject *UNUSED(self))
static bool bpy_blend_foreach_path_cb(BPathForeachPathData *bpath_data,
char *UNUSED(path_dst),
size_t UNUSED(path_dst_maxncpy),
const char *path_src)
{
PyObject *py_list = bpath_data->user_data;