From a86d3ed2914c0499ce4749ef377d3cb28a1d07e4 Mon Sep 17 00:00:00 2001 From: Min Yeol Lim Date: Wed, 22 Jul 2026 18:15:20 -0700 Subject: [PATCH 1/9] Fix symlink escape vulnerabilities in safe_copy and log file reading Harden file operations to prevent container escape attacks (CWE-59): - safe_copy(): Use O_EXCL for atomic temp file creation, add symlink checks before writing to dst_tmp and renaming to dst - Add safe_read_text(): Read files using O_NOFOLLOW to refuse symlinks These changes prevent an attacker in a profiled container from: 1. Overwriting arbitrary host files via symlink at the libasyncProfiler.so copy destination 2. Exfiltrating host file contents via symlink at the async-profiler log path Co-Authored-By: Claude Opus 4.5 --- gprofiler/profilers/java.py | 7 ++--- gprofiler/utils/fs.py | 60 ++++++++++++++++++++++++++++++++++++- 2 files changed, 62 insertions(+), 5 deletions(-) diff --git a/gprofiler/profilers/java.py b/gprofiler/profilers/java.py index 2e85486a1..f6db1e9e7 100644 --- a/gprofiler/profilers/java.py +++ b/gprofiler/profilers/java.py @@ -98,7 +98,7 @@ touch_path, wait_event, ) -from gprofiler.utils.fs import is_owned_by_root, is_rw_exec_dir, mkdir_owned_root, safe_copy +from gprofiler.utils.fs import is_owned_by_root, is_rw_exec_dir, mkdir_owned_root, safe_copy, safe_read_text from gprofiler.utils.perf import can_i_use_perf_events from gprofiler.utils.process import process_comm, search_proc_maps @@ -769,11 +769,10 @@ def _read_ap_log(self) -> str: if not os.path.exists(self._log_path_host): return "(log file doesn't exist)" - log = Path(self._log_path_host) - ap_log = log.read_text() + ap_log = safe_read_text(self._log_path_host) # clean immediately so we don't mix log messages from multiple invocations. # this is also what AP's profiler.sh does. - log.unlink() + Path(self._log_path_host).unlink() self._recreate_log() return ap_log diff --git a/gprofiler/utils/fs.py b/gprofiler/utils/fs.py index 1371e566f..b3217975d 100644 --- a/gprofiler/utils/fs.py +++ b/gprofiler/utils/fs.py @@ -17,6 +17,7 @@ import errno import os import shutil +import stat from pathlib import Path from secrets import token_hex from typing import Union @@ -27,16 +28,73 @@ from gprofiler.utils import remove_path, run_process +def _is_symlink_lstat(path: str) -> bool: + """Check if path is a symlink without following it.""" + try: + return stat.S_ISLNK(os.lstat(path).st_mode) + except FileNotFoundError: + return False + + def safe_copy(src: str, dst: str) -> None: """ Safely copies 'src' to 'dst'. Safely means that writing 'dst' is performed at a temporary location, and the file is then moved, making the filesystem-level change atomic. + + Security: Uses O_EXCL to atomically create the temp file, preventing symlink attacks where an + attacker plants a symlink to redirect writes to arbitrary locations. """ dst_tmp = f"{dst}.tmp" - shutil.copy(src, dst_tmp) + + # Remove existing tmp file if it's a regular file (from interrupted previous copy) + # If it's a symlink, refuse to proceed + if os.path.lexists(dst_tmp): + if _is_symlink_lstat(dst_tmp): + raise Exception(f"Refusing to copy to {dst_tmp}: path is a symlink") + os.unlink(dst_tmp) + + # O_EXCL ensures atomic creation - fails if anything exists at path (including symlinks) + # This closes TOCTOU race between the check above and the open + fd = os.open(dst_tmp, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) + try: + with open(src, "rb") as src_file: + with os.fdopen(fd, "wb") as dst_file: + shutil.copyfileobj(src_file, dst_file) + except Exception: + try: + os.unlink(dst_tmp) + except OSError: + pass + raise + + # Check dst is not a symlink before final rename + if _is_symlink_lstat(dst): + os.unlink(dst_tmp) + raise Exception(f"Refusing to rename to {dst}: path is a symlink") + os.rename(dst_tmp, dst) +def safe_read_text(path: str) -> str: + """ + Safely read text from a file, refusing to follow symlinks. + + Raises if path is a symlink. + """ + if _is_symlink_lstat(path): + raise Exception(f"Refusing to read {path}: path is a symlink") + + try: + fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW) + except OSError as e: + if e.errno == errno.ELOOP: + raise Exception(f"Refusing to read {path}: path is a symlink") + raise + + with os.fdopen(fd, "r") as f: + return f.read() + + def is_rw_exec_dir(path: Path) -> bool: """ Is 'path' rw and exec? From 37c222a1dc3bddfdaf73a0d4020422185dac639b Mon Sep 17 00:00:00 2001 From: Min Yeol Lim Date: Thu, 23 Jul 2026 10:49:15 -0700 Subject: [PATCH 2/9] Fix fd leak, preserve file permissions, and improve portability in safe_copy - Wrap fd with os.fdopen() before opening src to prevent fd leak if open(src) fails - Add shutil.copymode() to preserve source file permissions (e.g., executable bit for libasyncProfiler.so) - Use getattr(os, "O_NOFOLLOW", 0) for portability on platforms where O_NOFOLLOW is not available Co-Authored-By: Claude Opus 4.5 --- gprofiler/utils/fs.py | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/gprofiler/utils/fs.py b/gprofiler/utils/fs.py index b3217975d..02b544728 100644 --- a/gprofiler/utils/fs.py +++ b/gprofiler/utils/fs.py @@ -57,9 +57,11 @@ def safe_copy(src: str, dst: str) -> None: # This closes TOCTOU race between the check above and the open fd = os.open(dst_tmp, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) try: - with open(src, "rb") as src_file: - with os.fdopen(fd, "wb") as dst_file: - shutil.copyfileobj(src_file, dst_file) + # Wrap fd first to prevent leak if open(src) fails + with os.fdopen(fd, "wb") as dst_file, open(src, "rb") as src_file: + shutil.copyfileobj(src_file, dst_file) + # Preserve source file permissions (e.g., executable bit) + shutil.copymode(src, dst_tmp) except Exception: try: os.unlink(dst_tmp) @@ -85,7 +87,7 @@ def safe_read_text(path: str) -> str: raise Exception(f"Refusing to read {path}: path is a symlink") try: - fd = os.open(path, os.O_RDONLY | os.O_NOFOLLOW) + fd = os.open(path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0)) except OSError as e: if e.errno == errno.ELOOP: raise Exception(f"Refusing to read {path}: path is a symlink") From e7d696ad642b6e8d76625c9a0de8a0e6043dca34 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Jul 2026 02:27:05 +0000 Subject: [PATCH 3/9] Fix flaky Zing test: skip on network-related Docker build failures --- tests/conftest.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/tests/conftest.py b/tests/conftest.py index bb8264d97..431aff21d 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -427,7 +427,17 @@ def application_docker_image( ) if application_image_tag == "musl": pytest.xfail("This test does not work on aarch64 https://github.com/intel/gprofiler/issues/743") - image = build_image(docker_client, **application_docker_image_configs[image_name(runtime, application_image_tag)]) + try: + image = build_image( + docker_client, **application_docker_image_configs[image_name(runtime, application_image_tag)] + ) + except docker.errors.BuildError as e: + # Skip tests when Docker image build fails due to network issues (e.g., unavailable package repositories). + # This prevents transient network failures from turning into hard CI failures. + error_str = str(e) + if "Failed to fetch" in error_str or "Some index files failed to download" in error_str: + pytest.skip(f"Skipping: Docker image build failed due to unavailable package repository: {e}") + raise yield image # Clean up the test image after test completes to free disk space for subsequent tests From 46b7cfe2e205ec1ee60d7a556346af0179af9a11 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Jul 2026 02:29:26 +0000 Subject: [PATCH 4/9] Fix fd leak in safe_copy and safe_read_text when os.fdopen fails --- gprofiler/utils/fs.py | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/gprofiler/utils/fs.py b/gprofiler/utils/fs.py index 02b544728..40a1b8366 100644 --- a/gprofiler/utils/fs.py +++ b/gprofiler/utils/fs.py @@ -57,8 +57,16 @@ def safe_copy(src: str, dst: str) -> None: # This closes TOCTOU race between the check above and the open fd = os.open(dst_tmp, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) try: - # Wrap fd first to prevent leak if open(src) fails - with os.fdopen(fd, "wb") as dst_file, open(src, "rb") as src_file: + dst_file = os.fdopen(fd, "wb") + except Exception: + os.close(fd) + try: + os.unlink(dst_tmp) + except OSError: + pass + raise + try: + with dst_file, open(src, "rb") as src_file: shutil.copyfileobj(src_file, dst_file) # Preserve source file permissions (e.g., executable bit) shutil.copymode(src, dst_tmp) @@ -93,7 +101,12 @@ def safe_read_text(path: str) -> str: raise Exception(f"Refusing to read {path}: path is a symlink") raise - with os.fdopen(fd, "r") as f: + try: + f = os.fdopen(fd, "r") + except Exception: + os.close(fd) + raise + with f: return f.read() From dab1e1f203c6d1dcebd581b6bdb6569b5461e971 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Jul 2026 02:30:56 +0000 Subject: [PATCH 5/9] Improve safe_copy error handling and document O_NOFOLLOW platform behavior --- gprofiler/utils/fs.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/gprofiler/utils/fs.py b/gprofiler/utils/fs.py index 40a1b8366..fdad07d3f 100644 --- a/gprofiler/utils/fs.py +++ b/gprofiler/utils/fs.py @@ -53,9 +53,15 @@ def safe_copy(src: str, dst: str) -> None: raise Exception(f"Refusing to copy to {dst_tmp}: path is a symlink") os.unlink(dst_tmp) - # O_EXCL ensures atomic creation - fails if anything exists at path (including symlinks) - # This closes TOCTOU race between the check above and the open - fd = os.open(dst_tmp, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) + # O_EXCL ensures atomic creation - fails if anything exists at path (including symlinks). + # This closes TOCTOU race between the check above and the open. + # EEXIST means another process created the file after our delete - indicates a race or attack. + try: + fd = os.open(dst_tmp, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644) + except FileExistsError: + raise Exception( + f"Refusing to copy: {dst_tmp} was created unexpectedly (possible race condition or symlink attack)" + ) try: dst_file = os.fdopen(fd, "wb") except Exception: @@ -95,6 +101,9 @@ def safe_read_text(path: str) -> str: raise Exception(f"Refusing to read {path}: path is a symlink") try: + # O_NOFOLLOW makes open() fail with ELOOP if the path is a symlink (Linux-specific behaviour). + # On platforms without O_NOFOLLOW (e.g. Windows), we fall back to 0 and rely solely on the + # lstat check above. The target platform for this code is Linux, so O_NOFOLLOW is always set. fd = os.open(path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0)) except OSError as e: if e.errno == errno.ELOOP: From d873da744eda4c21c384892061f93e91fe43ce74 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Jul 2026 02:32:34 +0000 Subject: [PATCH 6/9] Simplify safe_read_text to use O_NOFOLLOW atomically without redundant lstat pre-check --- gprofiler/utils/fs.py | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/gprofiler/utils/fs.py b/gprofiler/utils/fs.py index fdad07d3f..f84e40cd8 100644 --- a/gprofiler/utils/fs.py +++ b/gprofiler/utils/fs.py @@ -95,15 +95,17 @@ def safe_read_text(path: str) -> str: """ Safely read text from a file, refusing to follow symlinks. + Uses O_NOFOLLOW so the kernel rejects symlinks atomically at open time (Linux). + On platforms without O_NOFOLLOW the flag falls back to 0 and the protection + is best-effort; the target platform for this code is Linux where O_NOFOLLOW + is always available. + Raises if path is a symlink. """ - if _is_symlink_lstat(path): - raise Exception(f"Refusing to read {path}: path is a symlink") - try: - # O_NOFOLLOW makes open() fail with ELOOP if the path is a symlink (Linux-specific behaviour). - # On platforms without O_NOFOLLOW (e.g. Windows), we fall back to 0 and rely solely on the - # lstat check above. The target platform for this code is Linux, so O_NOFOLLOW is always set. + # O_NOFOLLOW makes open() fail with ELOOP if the path is a symlink (Linux-specific behavior). + # On platforms without O_NOFOLLOW the flag is 0 and the call may follow symlinks; the target + # platform for this code is Linux, so O_NOFOLLOW is always available. fd = os.open(path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0)) except OSError as e: if e.errno == errno.ELOOP: From 5e3f80bfdc686305fe68ba626049c824b0ffd525 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Jul 2026 02:34:55 +0000 Subject: [PATCH 7/9] Clean up safe_copy and safe_read_text: simplify pre-cleanup, add O_NOFOLLOW constant, clarify rename behavior --- gprofiler/utils/fs.py | 24 ++++++++++++++++++------ 1 file changed, 18 insertions(+), 6 deletions(-) diff --git a/gprofiler/utils/fs.py b/gprofiler/utils/fs.py index f84e40cd8..9c7ba9d18 100644 --- a/gprofiler/utils/fs.py +++ b/gprofiler/utils/fs.py @@ -27,6 +27,11 @@ from gprofiler.platform import is_windows from gprofiler.utils import remove_path, run_process +# O_NOFOLLOW is always available on Linux (the target platform for this code). +# The getattr fallback to 0 covers non-Linux builds; on those platforms symlink +# protection in safe_read_text() is best-effort only. +_O_NOFOLLOW: int = getattr(os, "O_NOFOLLOW", 0) + def _is_symlink_lstat(path: str) -> bool: """Check if path is a symlink without following it.""" @@ -46,12 +51,16 @@ def safe_copy(src: str, dst: str) -> None: """ dst_tmp = f"{dst}.tmp" - # Remove existing tmp file if it's a regular file (from interrupted previous copy) - # If it's a symlink, refuse to proceed - if os.path.lexists(dst_tmp): - if _is_symlink_lstat(dst_tmp): + # Remove any leftover tmp file from a previous interrupted copy (regular files only). + # Refuse if the path is a symlink to prevent redirecting writes via a pre-planted symlink. + # O_EXCL below closes the TOCTOU race between this cleanup and the open. + try: + st = os.lstat(dst_tmp) + if stat.S_ISLNK(st.st_mode): raise Exception(f"Refusing to copy to {dst_tmp}: path is a symlink") os.unlink(dst_tmp) + except FileNotFoundError: + pass # Normal case: no leftover file # O_EXCL ensures atomic creation - fails if anything exists at path (including symlinks). # This closes TOCTOU race between the check above and the open. @@ -83,7 +92,10 @@ def safe_copy(src: str, dst: str) -> None: pass raise - # Check dst is not a symlink before final rename + # Best-effort check: refuse if dst is currently a symlink. + # os.rename() replaces the destination atomically (it does not follow dst symlinks), so even + # if an attacker races to plant a symlink between this check and the rename, the symlink itself + # would be replaced rather than its target being overwritten. This check adds defence-in-depth. if _is_symlink_lstat(dst): os.unlink(dst_tmp) raise Exception(f"Refusing to rename to {dst}: path is a symlink") @@ -106,7 +118,7 @@ def safe_read_text(path: str) -> str: # O_NOFOLLOW makes open() fail with ELOOP if the path is a symlink (Linux-specific behavior). # On platforms without O_NOFOLLOW the flag is 0 and the call may follow symlinks; the target # platform for this code is Linux, so O_NOFOLLOW is always available. - fd = os.open(path, os.O_RDONLY | getattr(os, "O_NOFOLLOW", 0)) + fd = os.open(path, os.O_RDONLY | _O_NOFOLLOW) except OSError as e: if e.errno == errno.ELOOP: raise Exception(f"Refusing to read {path}: path is a symlink") From fc624d9d932a08271efe1332345171fbd0822a00 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Jul 2026 02:36:35 +0000 Subject: [PATCH 8/9] Fix error messages and improve build log inspection for network error detection --- gprofiler/utils/fs.py | 6 +++--- tests/conftest.py | 9 ++++++--- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/gprofiler/utils/fs.py b/gprofiler/utils/fs.py index 9c7ba9d18..c83ed038e 100644 --- a/gprofiler/utils/fs.py +++ b/gprofiler/utils/fs.py @@ -57,7 +57,7 @@ def safe_copy(src: str, dst: str) -> None: try: st = os.lstat(dst_tmp) if stat.S_ISLNK(st.st_mode): - raise Exception(f"Refusing to copy to {dst_tmp}: path is a symlink") + raise Exception(f"Refusing to copy: temporary path {dst_tmp} is a symlink (possible attack)") os.unlink(dst_tmp) except FileNotFoundError: pass # Normal case: no leftover file @@ -98,7 +98,7 @@ def safe_copy(src: str, dst: str) -> None: # would be replaced rather than its target being overwritten. This check adds defence-in-depth. if _is_symlink_lstat(dst): os.unlink(dst_tmp) - raise Exception(f"Refusing to rename to {dst}: path is a symlink") + raise Exception(f"Refusing to copy: destination {dst} is a symlink (security restriction)") os.rename(dst_tmp, dst) @@ -121,7 +121,7 @@ def safe_read_text(path: str) -> str: fd = os.open(path, os.O_RDONLY | _O_NOFOLLOW) except OSError as e: if e.errno == errno.ELOOP: - raise Exception(f"Refusing to read {path}: path is a symlink") + raise Exception(f"Refusing to read {path}: symlinks are not allowed for security reasons") raise try: diff --git a/tests/conftest.py b/tests/conftest.py index 431aff21d..2acd38ae8 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -432,10 +432,13 @@ def application_docker_image( docker_client, **application_docker_image_configs[image_name(runtime, application_image_tag)] ) except docker.errors.BuildError as e: - # Skip tests when Docker image build fails due to network issues (e.g., unavailable package repositories). + # Skip tests when Docker image build fails due to network issues (e.g., unavailable package + # repositories). The APT "Failed to fetch" / "Some index files failed to download" messages + # appear in the build log stream entries, not in the top-level error reason. # This prevents transient network failures from turning into hard CI failures. - error_str = str(e) - if "Failed to fetch" in error_str or "Some index files failed to download" in error_str: + network_error_markers = ("Failed to fetch", "Some index files failed to download") + build_log_text = " ".join(str(entry) for entry in e.build_log) + if any(marker in build_log_text for marker in network_error_markers): pytest.skip(f"Skipping: Docker image build failed due to unavailable package repository: {e}") raise yield image From 3aab8fe8faccf370d6523d891f5a60999c5dc3cd Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Fri, 24 Jul 2026 02:39:12 +0000 Subject: [PATCH 9/9] Simplify dst_tmp cleanup: unlink directly and rely on O_EXCL for atomicity --- gprofiler/utils/fs.py | 12 ++++-------- 1 file changed, 4 insertions(+), 8 deletions(-) diff --git a/gprofiler/utils/fs.py b/gprofiler/utils/fs.py index c83ed038e..6de0239cd 100644 --- a/gprofiler/utils/fs.py +++ b/gprofiler/utils/fs.py @@ -51,19 +51,15 @@ def safe_copy(src: str, dst: str) -> None: """ dst_tmp = f"{dst}.tmp" - # Remove any leftover tmp file from a previous interrupted copy (regular files only). - # Refuse if the path is a symlink to prevent redirecting writes via a pre-planted symlink. - # O_EXCL below closes the TOCTOU race between this cleanup and the open. + # Remove any leftover tmp file from a previous interrupted copy. + # unlink() removes symlinks themselves (not their targets), so this is safe even if dst_tmp + # is a symlink; the subsequent O_EXCL open then creates the file fresh. try: - st = os.lstat(dst_tmp) - if stat.S_ISLNK(st.st_mode): - raise Exception(f"Refusing to copy: temporary path {dst_tmp} is a symlink (possible attack)") os.unlink(dst_tmp) except FileNotFoundError: pass # Normal case: no leftover file - # O_EXCL ensures atomic creation - fails if anything exists at path (including symlinks). - # This closes TOCTOU race between the check above and the open. + # O_EXCL ensures atomic creation - fails if anything exists at dst_tmp (including symlinks). # EEXIST means another process created the file after our delete - indicates a race or attack. try: fd = os.open(dst_tmp, os.O_WRONLY | os.O_CREAT | os.O_EXCL, 0o644)