Fix symlink escape vulnerabilities in safe_copy and log file reading

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
 

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
@@ -26,17 +27,108 @@
 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."""
+    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 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:
+        os.unlink(dst_tmp)
+    except FileNotFoundError:
+        pass  # Normal case: no leftover file
+
+    # 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)
+    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:
+        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)
+    except Exception:
+        try:
+            os.unlink(dst_tmp)
+        except OSError:
+            pass
+        raise
+
+    # 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 copy: destination {dst} is a symlink (security restriction)")
+
     os.rename(dst_tmp, dst)
 
 
+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.
+    """
+    try:
+        # 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 | _O_NOFOLLOW)
+    except OSError as e:
+        if e.errno == errno.ELOOP:
+            raise Exception(f"Refusing to read {path}: symlinks are not allowed for security reasons")
+        raise
+
+    try:
+        f = os.fdopen(fd, "r")
+    except Exception:
+        os.close(fd)
+        raise
+    with f:
+        return f.read()
+
+
 def is_rw_exec_dir(path: Path) -> bool:
     """
     Is 'path' rw and exec?

tests/conftest.py

@@ -427,7 +427,20 @@ 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).  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.
+        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
 
     # Clean up the test image after test completes to free disk space for subsequent tests