Fix #139585: Blender could erase OS root

Temporary directory handling had a logical error, assuming the
"session" temporary directory was owned and created by Blender
and could be recursively removed on exit.

However, it's possible creating the session sub-directory fails,
in that case the temporary directory was used for the "session".
This meant setting `C:\` as the temporary directory in the preferences
would attempt to recursively remove `C:\` on exit.

Resolve with the following changes:

- Only perform a recursive removal on the temporary directory
  if a session sub-directory was created.

- If the creating the user-preferences temporary "session" sub-directory
  fails fall back to the systems temporary directory and try to
  create the "session" directory there.

  Previously this was only done if the preference path didn't exist.
  The preferences path was still used if it existed but couldn't be
  written to.

Include a test to ensure this is working as expected.

Ref !144042
This commit is contained in:
Campbell Barton
2025-08-06 23:13:58 +00:00
parent 21cbdab34a
commit 87c4f47312
4 changed files with 273 additions and 14 deletions

View File

@@ -70,6 +70,11 @@ static struct {
char temp_dirname_base[FILE_MAX];
/** Volatile temporary directory (owned by Blender, removed on exit). */
char temp_dirname_session[FILE_MAX];
/**
* True when this is a sub-directory owned & created by Blender,
* false when a session directory couldn't be created - in this case don't delete it.
*/
bool temp_dirname_session_can_be_deleted;
} g_app{};
/** \} */
@@ -1144,16 +1149,19 @@ void BKE_appdir_app_templates(ListBase *templates)
* \param tempdir_maxncpy: The size of the \a tempdir buffer.
* \param userdir: Directory specified in user preferences (may be nullptr).
* note that by default this is an empty string, only use when non-empty.
*
* \return true if `userdir` is used.
*/
static void where_is_temp(char *tempdir, const size_t tempdir_maxncpy, const char *userdir)
static bool where_is_temp(char *tempdir, const size_t tempdir_maxncpy, const char *userdir)
{
if (userdir && BLI_temp_directory_path_copy_if_valid(tempdir, tempdir_maxncpy, userdir)) {
return;
return true;
}
BLI_temp_directory_path_get(tempdir, tempdir_maxncpy);
return false;
}
static void tempdir_session_create(char *tempdir_session,
static bool tempdir_session_create(char *tempdir_session,
const size_t tempdir_session_maxncpy,
const char *tempdir)
{
@@ -1182,15 +1190,12 @@ static void tempdir_session_create(char *tempdir_session,
if (BLI_is_dir(tempdir_session)) {
BLI_path_slash_ensure(tempdir_session, tempdir_session_maxncpy);
/* Success. */
return;
return true;
}
}
CLOG_WARN(&LOG,
"Could not generate a temp file name for '%s', falling back to '%s'",
tempdir_session,
tempdir);
BLI_strncpy(tempdir_session, tempdir, tempdir_session_maxncpy);
CLOG_WARN(&LOG, "Could not generate a temp file name for '%s'", tempdir_session);
return false;
}
void BKE_tempdir_init(const char *userdir)
@@ -1200,13 +1205,44 @@ void BKE_tempdir_init(const char *userdir)
* Sets #g_app.temp_dirname_session to a #mkdtemp
* generated sub-dir of #g_app.temp_dirname_base. */
where_is_temp(g_app.temp_dirname_base, sizeof(g_app.temp_dirname_base), userdir);
/* Clear existing temp dir, if needed. */
BKE_tempdir_session_purge();
/* Now that we have a valid temp dir, add system-generated unique sub-dir. */
tempdir_session_create(
g_app.temp_dirname_session, sizeof(g_app.temp_dirname_session), g_app.temp_dirname_base);
/* Perform two passes, the first pass for the user preference path,
* then a second pass if the the preferences failed to create the *session* sub-directory.
*
* This avoid problems if the preferences points to a path without write access,
* `C:\` or `/` for example. */
g_app.temp_dirname_session_can_be_deleted = false;
for (int pass = 0; pass < 2; pass += 1) {
const bool from_userdir = where_is_temp(
g_app.temp_dirname_base, sizeof(g_app.temp_dirname_base), pass == 0 ? userdir : nullptr);
/* Now that we have a valid temp dir, add system-generated unique sub-dir. */
if (tempdir_session_create(g_app.temp_dirname_session,
sizeof(g_app.temp_dirname_session),
g_app.temp_dirname_base))
{
/* Created the session sub-directory. */
g_app.temp_dirname_session_can_be_deleted = true;
break;
}
/* Only perform the second pass if the `userdir` was used
* and failed to created the sub-directory. */
if (from_userdir == false) {
break;
}
}
if (UNLIKELY(g_app.temp_dirname_session_can_be_deleted == false)) {
/* This should practically never happen as either the preferences or the systems
* default temporary directory should be usable, if not, use the base directory and warn. */
STRNCPY(g_app.temp_dirname_session, g_app.temp_dirname_base);
CLOG_WARN(&LOG,
"Could not generate a temp session subdirectory, falling back to '%s'",
g_app.temp_dirname_base);
}
}
const char *BKE_tempdir_session()
@@ -1221,6 +1257,11 @@ const char *BKE_tempdir_base()
void BKE_tempdir_session_purge()
{
if (g_app.temp_dirname_session_can_be_deleted == false) {
/* It's possible this path references an arbitrary location
* in that case *never* recursively remove, see: #139585. */
return;
}
if (g_app.temp_dirname_session[0] && BLI_is_dir(g_app.temp_dirname_session)) {
BLI_delete(g_app.temp_dirname_session, true, true);
}

View File

@@ -226,6 +226,11 @@ endif()
# PY API TESTS
# ------------------------------------------------------------------------------
add_blender_test(
script_pyapi_bpy_app_tempdir
--python ${CMAKE_CURRENT_LIST_DIR}/bl_pyapi_bpy_app_tempdir.py
)
add_blender_test(
script_pyapi_bpy_path
--python ${CMAKE_CURRENT_LIST_DIR}/bl_pyapi_bpy_path.py

View File

@@ -0,0 +1,212 @@
# SPDX-FileCopyrightText: 2025 Blender Authors
#
# SPDX-License-Identifier: GPL-2.0-or-later
# ./blender.bin --background --factory-startup --python tests/python/bl_tempfile.py -- --verbose
# NOTE(ideasman42):
#
# - Creating a directory without write permissions across all supported platforms
# is it not trivial, for the purpose of these tests simply create a file as a way of
# pointing to a temporary path which can't have sub-directories created under it.
#
# - Internally, when no temporary path is set the hard coded path `/tmp/` is used.
# For the purpose of the tests being complete - control over this path would be needed too.
#
# - When changing the temp directory & on exit
# Blender's *session* temp directory is recursively removed.
#
# These tests were added as part of a fix for a serious flaw (see #144042)
# which would recursively delete the users `C:\` since the tests aren't sand-boxed
# all of the following tests reference paths within a newly creating temporary directory
# instead of referencing root paths to avoid risks for developers who run tests.
#
# - A related test exists `BLI_tempfile_test.cc` however this doesn't deal with
# Blender's user preferences and fallbacks used in Blender.
#
__all__ = (
"main",
)
import os
import unittest
import tempfile
is_win32 = os.name == "nt"
TEMP_ENV = "TEMP" if is_win32 else "TMPDIR"
# TODO: remove this. Since it's checked, it must be removed!
if os.environ.get("TMP") is not None:
del os.environ["TMP"]
BASE_DIR = ""
def system_temp_set(path: str) -> None:
os.environ[TEMP_ENV] = path
def prefs_temp_set(path: str) -> None:
import bpy # type: ignore
bpy.context.preferences.filepaths.temporary_directory = path
def prefs_temp_get() -> str:
import bpy
result = bpy.context.preferences.filepaths.temporary_directory
assert isinstance(result, str)
return result
def blender_tempdir_session_get() -> str:
import bpy
result = bpy.app.tempdir
assert isinstance(result, str)
return result
def empty_file(path: str) -> None:
with open(path, 'wb') as _fh:
pass
def commonpath_safe(paths: list[str]) -> str:
if is_win32:
try:
return os.path.commonpath(paths)
except ValueError:
# Workaround error on Windows.
# ValueError: Paths don't have the same drive
return ""
return os.path.commonpath(paths)
class TestTempDir(unittest.TestCase):
def setUp(self) -> None:
print(prefs_temp_get())
assert prefs_temp_get() == ""
system_temp_set("")
prefs_temp_set("")
def tearDown(self) -> None:
prefs_temp_set("")
system_temp_set("")
def test_fallback(self) -> None:
# Set a file for preferences & system temp, ensure neither are used.
with tempfile.TemporaryDirectory() as tempdir:
temp_sys = os.path.join(tempdir, "a_sys")
temp_bpy = os.path.join(tempdir, "b_bpy")
empty_file(temp_sys)
empty_file(temp_bpy)
system_temp_set(temp_sys)
prefs_temp_set(temp_bpy)
temp_session = blender_tempdir_session_get()
# Ensure neither are used.
# This will try to use `/tmp/`.
commonpath_test = commonpath_safe([temp_sys, temp_session])
self.assertFalse(commonpath_test and os.path.samefile(temp_sys, commonpath_test))
commonpath_test = commonpath_safe([temp_bpy, temp_session])
self.assertFalse(commonpath_test and os.path.samefile(temp_bpy, commonpath_test))
def test_system(self) -> None:
# Set an file as the preferences temp directory, ensure the system path is used.
with tempfile.TemporaryDirectory() as tempdir:
temp_sys = os.path.join(tempdir, "a_sys")
temp_bpy = os.path.join(tempdir, "b_bpy")
os.mkdir(temp_sys)
empty_file(temp_bpy)
system_temp_set(temp_sys)
prefs_temp_set(temp_bpy)
temp_session = blender_tempdir_session_get()
self.assertTrue(os.path.samefile(temp_sys, os.path.commonpath([temp_sys, temp_session])))
def test_prefs(self) -> None:
# Set an file as the system temp directory, ensure the preferences path is used.
with tempfile.TemporaryDirectory() as tempdir:
temp_sys = os.path.join(tempdir, "a_sys")
temp_bpy = os.path.join(tempdir, "b_bpy")
empty_file(temp_sys)
os.mkdir(temp_bpy)
system_temp_set(temp_sys)
prefs_temp_set(temp_bpy)
temp_session = blender_tempdir_session_get()
self.assertTrue(os.path.samefile(temp_bpy, os.path.commonpath([temp_bpy, temp_session])))
def test_system_to_prefs(self) -> None:
with tempfile.TemporaryDirectory() as tempdir:
temp_sys = os.path.join(tempdir, "a_sys")
temp_bpy = os.path.join(tempdir, "b_bpy")
os.mkdir(temp_sys)
empty_file(temp_bpy)
system_temp_set(temp_sys)
prefs_temp_set(temp_bpy)
temp_session = blender_tempdir_session_get()
self.assertTrue(os.path.samefile(temp_sys, os.path.commonpath([temp_sys, temp_session])))
# Now set the preferences and ensure the previous directory gets purged and the new one set.
os.unlink(temp_bpy)
os.mkdir(temp_bpy)
# Ensure the preferences path is now used.
prefs_temp_set(temp_bpy)
self.assertFalse(os.path.exists(temp_session))
temp_session = blender_tempdir_session_get()
self.assertTrue(os.path.samefile(temp_bpy, os.path.commonpath([temp_bpy, temp_session])))
def test_prefs_to_system(self) -> None:
with tempfile.TemporaryDirectory() as tempdir:
temp_sys = os.path.join(tempdir, "a_sys")
temp_bpy = os.path.join(tempdir, "b_bpy")
empty_file(temp_sys)
os.mkdir(temp_bpy)
system_temp_set(temp_sys)
prefs_temp_set(temp_bpy)
temp_session = blender_tempdir_session_get()
self.assertTrue(os.path.samefile(temp_bpy, os.path.commonpath([temp_bpy, temp_session])))
# Now set the preferences and ensure the previous directory gets purged and the new one set.
os.unlink(temp_sys)
os.mkdir(temp_sys)
# Ensure the system path is now used.
prefs_temp_set(temp_sys)
self.assertFalse(os.path.exists(temp_session))
temp_session = blender_tempdir_session_get()
self.assertTrue(os.path.samefile(temp_sys, os.path.commonpath([temp_sys, temp_session])))
def main():
import sys
unittest.main(argv=[__file__] + (sys.argv[sys.argv.index("--") + 1:] if "--" in sys.argv else []))
if __name__ == "__main__":
main()

View File

@@ -28,6 +28,7 @@ PATHS: tuple[tuple[str, tuple[Any, ...], dict[str, str]], ...] = (
("scripts/modules/_bpy_internal/freedesktop.py", (), {}),
("source/blender/nodes/intern/discover_nodes.py", (), {}),
("tests/python/bl_keymap_validate.py", (), {}),
("tests/python/bl_pyapi_bpy_app_tempdir.py", (), {}),
("tests/utils/blender_headless.py", (), {}),
("tools/check_blender_release/", (), {}),
("tools/check_docs/", (), {}),