From 9284887c7b17fea75a5c7be22828a78e3951faa1 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 25 May 2026 17:14:42 +0200 Subject: [PATCH 1/2] fix(path): Avoid following symlinks during recursive cleanup A symlinked .run entry in a shared or pre-existing database directory could pass the stat-based directory checks used by old-run processing. That allowed cleanup to traverse attacker-controlled symlink targets and delete files outside the SDK cache. Detect symlinks explicitly and avoid recursing through them during cleanup, so old run processing removes only real cache directories and never follows a planted .run symlink. Co-Authored-By: OpenAI Codex --- src/path/sentry_path.c | 2 +- src/path/sentry_path_unix.c | 14 ++++++++- src/path/sentry_path_windows.c | 12 ++++++++ src/sentry_database.c | 2 +- src/sentry_path.h | 5 ++++ tests/unit/test_cache.c | 52 ++++++++++++++++++++++++++++++++++ tests/unit/test_path.c | 46 ++++++++++++++++++++++++++++++ tests/unit/tests.inc | 2 ++ 8 files changed, 132 insertions(+), 3 deletions(-) diff --git a/src/path/sentry_path.c b/src/path/sentry_path.c index 2ec8631d4d..2f47e550c9 100644 --- a/src/path/sentry_path.c +++ b/src/path/sentry_path.c @@ -69,7 +69,7 @@ sentry__path_eq(const sentry_path_t *path_a, const sentry_path_t *path_b) int sentry__path_remove_all(const sentry_path_t *path) { - if (sentry__path_is_dir(path)) { + if (sentry__path_is_dir(path) && !sentry__path_is_symlink(path)) { sentry_pathiter_t *piter = sentry__path_iter_directory(path); if (piter) { const sentry_path_t *p; diff --git a/src/path/sentry_path_unix.c b/src/path/sentry_path_unix.c index 752025bf14..e6442d8f1e 100644 --- a/src/path/sentry_path_unix.c +++ b/src/path/sentry_path_unix.c @@ -229,6 +229,18 @@ sentry__path_is_file(const sentry_path_t *path) return stat(path->path, &buf) == 0 && S_ISREG(buf.st_mode); } +bool +sentry__path_is_symlink(const sentry_path_t *path) +{ +#if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) + (void)path; + return false; +#else + struct stat buf; + return lstat(path->path, &buf) == 0 && S_ISLNK(buf.st_mode); +#endif +} + size_t sentry__path_get_size(const sentry_path_t *path) { @@ -309,7 +321,7 @@ int sentry__path_remove(const sentry_path_t *path) { int status; - if (!sentry__path_is_dir(path)) { + if (!sentry__path_is_dir(path) || sentry__path_is_symlink(path)) { EINTR_RETRY(unlink(path->path), &status); if (status == 0) { return 0; diff --git a/src/path/sentry_path_windows.c b/src/path/sentry_path_windows.c index 5ccda3301e..5d62b3b468 100644 --- a/src/path/sentry_path_windows.c +++ b/src/path/sentry_path_windows.c @@ -351,6 +351,18 @@ sentry__path_is_file(const sentry_path_t *path) return _wstat(path_w, &buf) == 0 && S_ISREG(buf.st_mode); } +bool +sentry__path_is_symlink(const sentry_path_t *path) +{ + wchar_t *path_w = path->path_w; + if (!path_w) { + return false; + } + const DWORD attributes = GetFileAttributesW(path_w); + return attributes != INVALID_FILE_ATTRIBUTES + && (attributes & FILE_ATTRIBUTE_REPARSE_POINT) != 0; +} + size_t sentry__path_get_size(const sentry_path_t *path) { diff --git a/src/sentry_database.c b/src/sentry_database.c index ced802ebfc..278fa70134 100644 --- a/src/sentry_database.c +++ b/src/sentry_database.c @@ -609,7 +609,7 @@ sentry__process_old_runs(const sentry_options_t *options, uint64_t last_crash) while ((run_dir = sentry__pathiter_next(db_iter)) != NULL) { // skip over other files such as the saved consent or the last_crash // timestamp - if (!sentry__path_is_dir(run_dir)) { + if (!sentry__path_is_dir(run_dir) || sentry__path_is_symlink(run_dir)) { continue; } diff --git a/src/sentry_path.h b/src/sentry_path.h index f8140cd1e2..f21d7c66fc 100644 --- a/src/sentry_path.h +++ b/src/sentry_path.h @@ -148,6 +148,11 @@ bool sentry__path_ends_with(const sentry_path_t *path, const char *suffix); */ bool sentry__path_is_dir(const sentry_path_t *path); +/** + * Return whether the path refers to a symlink. + */ +bool sentry__path_is_symlink(const sentry_path_t *path); + /** * Return whether the path refers to a regular file. */ diff --git a/tests/unit/test_cache.c b/tests/unit/test_cache.c index 265e6c133d..8d078a5b0f 100644 --- a/tests/unit/test_cache.c +++ b/tests/unit/test_cache.c @@ -12,6 +12,7 @@ #ifdef SENTRY_PLATFORM_WINDOWS # include #elif !defined(SENTRY_PLATFORM_NX) && !defined(SENTRY_PLATFORM_PS) +# include # include #endif @@ -439,6 +440,57 @@ SENTRY_TEST(cache_prune_siblings) sentry_close(); } +SENTRY_TEST(cache_symlink_run) +{ +#if !defined(SENTRY_PLATFORM_UNIX) || defined(SENTRY_PLATFORM_NX) \ + || defined(SENTRY_PLATFORM_PS) + SKIP_TEST(); +#else + SENTRY_TEST_OPTIONS_NEW(options); + sentry_options_set_dsn(options, "https://foo@sentry.invalid/42"); + sentry_init(options); + + sentry_path_t *target_dir + = sentry__path_from_str(SENTRY_TEST_PATH_PREFIX ".old-run-target"); + TEST_ASSERT(!!target_dir); + sentry_path_t *target_file = sentry__path_join_str(target_dir, "important"); + TEST_ASSERT(!!target_file); + sentry_path_t *linked_run_dir + = sentry__path_join_str(options->database_path, "malicious.run"); + TEST_ASSERT(!!linked_run_dir); + sentry_path_t *linked_run_lock + = sentry__path_append_str(linked_run_dir, ".lock"); + TEST_ASSERT(!!linked_run_lock); + + // Model an old run entry like: + // /malicious.run -> /.old-run-target + // The target stays outside the database so following the link would remove + // files outside SDK-owned cache directories. + sentry__path_remove(linked_run_dir); + sentry__path_remove(linked_run_lock); + TEST_ASSERT(sentry__path_remove_all(target_dir) == 0); + TEST_ASSERT(sentry__path_create_dir_all(target_dir) == 0); + TEST_ASSERT(sentry__path_write_buffer(target_file, "keep", 4) == 0); + sentry_path_t *target_dir_abs = sentry__path_absolute(target_dir); + TEST_ASSERT(!!target_dir_abs); + TEST_ASSERT(symlink(target_dir_abs->path, linked_run_dir->path) == 0); + + sentry__process_old_runs(options, 0); + + TEST_CHECK(sentry__path_is_file(target_file)); + + sentry__path_remove(linked_run_dir); + sentry__path_remove(linked_run_lock); + sentry__path_remove_all(target_dir); + sentry__path_free(linked_run_lock); + sentry__path_free(linked_run_dir); + sentry__path_free(target_dir_abs); + sentry__path_free(target_file); + sentry__path_free(target_dir); + sentry_close(); +#endif +} + SENTRY_TEST(cache_consent_revoked) { #if defined(SENTRY_PLATFORM_NX) || defined(SENTRY_PLATFORM_PS) diff --git a/tests/unit/test_path.c b/tests/unit/test_path.c index 2f0ff06df8..6b782e8285 100644 --- a/tests/unit/test_path.c +++ b/tests/unit/test_path.c @@ -2,6 +2,11 @@ #include "sentry_string.h" #include "sentry_testsupport.h" +#if defined(SENTRY_PLATFORM_UNIX) && !defined(SENTRY_PLATFORM_NX) \ + && !defined(SENTRY_PLATFORM_PS) +# include +#endif + SENTRY_TEST(recursive_paths) { sentry_path_t *base = sentry__path_from_str(SENTRY_TEST_PATH_PREFIX ".foo"); @@ -247,6 +252,47 @@ SENTRY_TEST(path_directory) sentry__path_free(path_2); } +SENTRY_TEST(path_remove_all_symlink) +{ +#if !defined(SENTRY_PLATFORM_UNIX) || defined(SENTRY_PLATFORM_NX) \ + || defined(SENTRY_PLATFORM_PS) + SKIP_TEST(); +#else + sentry_path_t *base + = sentry__path_from_str(SENTRY_TEST_PATH_PREFIX ".remove-all"); + TEST_ASSERT(!!base); + sentry_path_t *target = sentry__path_join_str(base, "target"); + TEST_ASSERT(!!target); + sentry_path_t *target_file = sentry__path_join_str(target, "important"); + TEST_ASSERT(!!target_file); + sentry_path_t *link = sentry__path_join_str(base, "link"); + TEST_ASSERT(!!link); + + // Model a recursive deletion target like: + // .remove-all/link -> .remove-all/target + // Removing the link must not recurse into the target directory. + TEST_ASSERT(sentry__path_remove_all(base) == 0); + TEST_ASSERT(sentry__path_create_dir_all(target) == 0); + TEST_ASSERT(sentry__path_write_buffer(target_file, "keep", 4) == 0); + sentry_path_t *target_abs = sentry__path_absolute(target); + TEST_ASSERT(!!target_abs); + TEST_ASSERT(symlink(target_abs->path, link->path) == 0); + TEST_CHECK(sentry__path_is_symlink(link)); + + TEST_ASSERT(sentry__path_remove_all(link) == 0); + + TEST_CHECK(sentry__path_is_file(target_file)); + TEST_CHECK(!sentry__path_is_dir(link)); + + sentry__path_remove_all(base); + sentry__path_free(link); + sentry__path_free(target_file); + sentry__path_free(target_abs); + sentry__path_free(target); + sentry__path_free(base); +#endif +} + SENTRY_TEST(path_mtime) { #if defined(SENTRY_PLATFORM_NX) diff --git a/tests/unit/tests.inc b/tests/unit/tests.inc index b1a2248317..6bfdf76025 100644 --- a/tests/unit/tests.inc +++ b/tests/unit/tests.inc @@ -66,6 +66,7 @@ XX(cache_max_size) XX(cache_max_size_and_age) XX(cache_prune_siblings) XX(cache_remove_siblings) +XX(cache_symlink_run) XX(cache_write_minidump) XX(cache_write_raw_with_minidump) XX(capture_minidump_basic) @@ -225,6 +226,7 @@ XX(path_joining_unix) XX(path_joining_windows) XX(path_mtime) XX(path_relative_filename) +XX(path_remove_all_symlink) XX(path_rename) XX(path_unique) XX(percent_decode_basic) From 745695455f666546b19c69ce51fd8660a708e9a7 Mon Sep 17 00:00:00 2001 From: J-P Nurmi Date: Mon, 25 May 2026 17:29:54 +0200 Subject: [PATCH 2/2] Update CHANGELOG.md --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c79f5b032..59ed3eb9d2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - Native/macOS: fix thread stack descriptor. ([#1726](https://github.com/getsentry/sentry-native/pull/1726)) - Cap rate-limit retry-after values at 24 hours to prevent a MITM-provided response from disabling event delivery for the process lifetime. ([#1744](https://github.com/getsentry/sentry-native/pull/1744)) - Native: validate ELF header entry sizes. ([#1746](https://github.com/getsentry/sentry-native/pull/1746)) +- Prevent database cleanup from following symlinks in run and cache directories. ([#1751](https://github.com/getsentry/sentry-native/pull/1751)) ## 0.14.2