Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
- Native/macOS: honor the `system_crash_reporter_enabled` option. ([#1743](https://github.com/getsentry/sentry-native/pull/1743))
- 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))
- Structured logs: respect printf argument widths when extracting log parameters to avoid stack-data disclosure and corrupted attributes on 32-bit platforms. ([#1752](https://github.com/getsentry/sentry-native/pull/1752))

## 0.14.2
Expand Down
2 changes: 1 addition & 1 deletion src/path/sentry_path.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
14 changes: 13 additions & 1 deletion src/path/sentry_path_unix.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down Expand Up @@ -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;
Expand Down
12 changes: 12 additions & 0 deletions src/path/sentry_path_windows.c
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
2 changes: 1 addition & 1 deletion src/sentry_database.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}

Expand Down
5 changes: 5 additions & 0 deletions src/sentry_path.h
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
52 changes: 52 additions & 0 deletions tests/unit/test_cache.c
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
#ifdef SENTRY_PLATFORM_WINDOWS
# include <windows.h>
#elif !defined(SENTRY_PLATFORM_NX) && !defined(SENTRY_PLATFORM_PS)
# include <unistd.h>
# include <utime.h>
#endif

Expand Down Expand Up @@ -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:
// <database>/malicious.run -> <cwd>/.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)
Expand Down
46 changes: 46 additions & 0 deletions tests/unit/test_path.c
Original file line number Diff line number Diff line change
Expand Up @@ -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 <unistd.h>
#endif

SENTRY_TEST(recursive_paths)
{
sentry_path_t *base = sentry__path_from_str(SENTRY_TEST_PATH_PREFIX ".foo");
Expand Down Expand Up @@ -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)
Expand Down
2 changes: 2 additions & 0 deletions tests/unit/tests.inc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -227,6 +228,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)
Expand Down
Loading