From df4ca8d5f6bf4988d315f1d59337c51550260b42 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Thu, 16 Apr 2026 02:35:46 +0000 Subject: [PATCH] =?UTF-8?q?=F0=9F=9B=A1=EF=B8=8F=20Sentinel:=20[HIGH]=20Fi?= =?UTF-8?q?x=20insecure=20temporary=20file=20creation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Vulnerability: Predictable temporary file names (`path + ".tmp"`) were being used with `os.WriteFile` in `os.TempDir()` (a shared directory) to perform atomic file writes. Impact: A local attacker could launch a symlink attack by pre-creating the predictable `.tmp` file as a symlink pointing to an arbitrary file. When Prism runs and `os.WriteFile` executes, it follows the symlink, potentially overwriting the arbitrary file with Prism's cache data using the application user's privileges. Fix: Replaced `os.WriteFile` with `os.CreateTemp`, which correctly passes the `O_EXCL` flag when opening the file. This ensures the temporary file does not already exist, mitigating the symlink attack. We also create the file in the same directory as the target path to ensure `os.Rename` won't fail due to moving across filesystems. The temporary file is properly closed before calling `os.Rename` to avoid open-file locking errors. Co-authored-by: himattm <6266621+himattm@users.noreply.github.com> --- .jules/sentinel.md | 4 ++++ internal/burnrate/burnrate.go | 22 ++++++++++++++++++++-- internal/sparkline/sparkline.go | 27 ++++++++++++++++++++++++--- 3 files changed, 48 insertions(+), 5 deletions(-) create mode 100644 .jules/sentinel.md diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..bea515d --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-05-24 - [Insecure Temporary File Creation] +**Vulnerability:** Predictable temporary file names (e.g., `path + ".tmp"`) were used with `os.WriteFile` in shared directories like `os.TempDir()`. This allows symlink attacks where a malicious user could pre-create the predictable symlink, causing `os.WriteFile` to overwrite arbitrary files the application user has access to. +**Learning:** `os.WriteFile` follows symlinks. When performing atomic file writes via temporary files, predictable names should not be used in shared temporary directories. +**Prevention:** Always use `os.CreateTemp` to generate unpredictable, securely-created temporary files (it uses `O_EXCL` when opening) before writing data and subsequently calling `os.Rename`. diff --git a/internal/burnrate/burnrate.go b/internal/burnrate/burnrate.go index 8cb680c..8d0bb39 100644 --- a/internal/burnrate/burnrate.go +++ b/internal/burnrate/burnrate.go @@ -64,10 +64,28 @@ func LoadOrCreateSnapshotAt(sessionID string, currentCost float64, now time.Time return nil, false, err } - tmpPath := path + ".tmp" - if err := os.WriteFile(tmpPath, data, 0644); err != nil { + // Use os.CreateTemp for secure temporary file creation + tmpFile, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+"-*") + if err != nil { return nil, false, err } + tmpPath := tmpFile.Name() + + if _, err := tmpFile.Write(data); err != nil { + tmpFile.Close() + os.Remove(tmpPath) + return nil, false, err + } + if err := tmpFile.Close(); err != nil { + os.Remove(tmpPath) + return nil, false, err + } + + if err := os.Chmod(tmpPath, 0644); err != nil { + os.Remove(tmpPath) + return nil, false, err + } + if err := os.Rename(tmpPath, path); err != nil { os.Remove(tmpPath) return nil, false, err diff --git a/internal/sparkline/sparkline.go b/internal/sparkline/sparkline.go index f4c755c..1deace6 100644 --- a/internal/sparkline/sparkline.go +++ b/internal/sparkline/sparkline.go @@ -138,11 +138,32 @@ func Save(sessionID, metric string, b *Buffer) { if err != nil { return } - tmp := path + ".tmp" - if err := os.WriteFile(tmp, data, 0644); err != nil { + + // Use os.CreateTemp for secure temporary file creation + tmpFile, err := os.CreateTemp(filepath.Dir(path), filepath.Base(path)+"-*") + if err != nil { + return + } + tmp := tmpFile.Name() + + if _, err := tmpFile.Write(data); err != nil { + tmpFile.Close() + os.Remove(tmp) + return + } + if err := tmpFile.Close(); err != nil { + os.Remove(tmp) + return + } + + if err := os.Chmod(tmp, 0644); err != nil { + os.Remove(tmp) return } - os.Rename(tmp, path) + + if err := os.Rename(tmp, path); err != nil { + os.Remove(tmp) + } } // PushAndSave is a convenience that loads, pushes, saves, and returns the buffer