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