From d30641f87a2ec986fb76aee909b6015a15f8ec19 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 30 Mar 2026 06:19:40 +0000 Subject: [PATCH] Fix command injection vulnerability in toolExists Replaced the vulnerable string-interpolated shell wrapper execution (`shell("/usr/bin/which \(tool)")`) with a secure, direct invocation of the `/usr/bin/which` binary using `Foundation.Process`, passing the `tool` variable strictly via the `arguments` array. Assigned `FileHandle.nullDevice` to output pipes to avoid unnecessary allocations. Co-authored-by: acebytes <2820910+acebytes@users.noreply.github.com> --- .jules/sentinel.md | 4 ++++ Sources/Cacheout/Models/CacheCategory.swift | 16 ++++++++++++++-- 2 files changed, 18 insertions(+), 2 deletions(-) create mode 100644 .jules/sentinel.md diff --git a/.jules/sentinel.md b/.jules/sentinel.md new file mode 100644 index 0000000..395ab52 --- /dev/null +++ b/.jules/sentinel.md @@ -0,0 +1,4 @@ +## 2024-05-24 - String Interpolation in Shell Commands +**Vulnerability:** Command injection vulnerability identified in `toolExists` within `CacheCategory.swift`, which utilized a shell wrapper (`/bin/bash -c`) with string interpolation to pass dynamic arguments: `shell("/usr/bin/which \(tool)")`. +**Learning:** String interpolation combined with shell wrappers allows arbitrary command injection if the interpolated variable contains shell metacharacters. Even inside `toolExists`, where `requiresTool` inputs seemed controlled internally, the code pattern itself was inherently unsafe. +**Prevention:** Avoid using shell wrappers (`/bin/bash -c`) and string interpolation for executing dynamic commands. Always use direct execution of the binary using `Process` with arguments safely separated into the `process.arguments` array. diff --git a/Sources/Cacheout/Models/CacheCategory.swift b/Sources/Cacheout/Models/CacheCategory.swift index 7b3d942..818f93d 100644 --- a/Sources/Cacheout/Models/CacheCategory.swift +++ b/Sources/Cacheout/Models/CacheCategory.swift @@ -186,8 +186,20 @@ struct CacheCategory: Identifiable, Hashable { } private func toolExists(_ tool: String) -> Bool { - let result = shell("/usr/bin/which \(tool)") - return result != nil && !result!.isEmpty + let process = Process() + + process.executableURL = URL(fileURLWithPath: "/usr/bin/which") + process.arguments = [tool] + process.standardOutput = FileHandle.nullDevice + process.standardError = FileHandle.nullDevice + + do { + try process.run() + process.waitUntilExit() + return process.terminationStatus == 0 + } catch { + return false + } } private func runProbe(_ command: String) -> String? {