# Patch Analysis: CVE-2026-60102 (Horde VFS Smb command injection)

## Fix Commit

- **Repository:** https://github.com/horde/Vfs
- **Fixed commit:** `41f74b4acfc144e09013d04dd121e0a5da808361`
- **Parent (vulnerable) commit:** `994f4a46fd76ea44eae2fe839216bb273ce49080`
- **Released as:** Horde VFS `v3.0.1`
- **Files changed:** `lib/Horde/Vfs/Smb.php` (only)
- **Test added:** `test/Unit/SmbCommandInjectionTest.php` (regression tests for HF-01)

## What the Fix Changes

The patch makes two coordinated changes in `lib/Horde/Vfs/Smb.php`:

1. **Eliminates the shell-string execution path.**
   - `Horde_Vfs_Smb::_execute()` now receives an **argv-style array** and passes it directly to `proc_open()`.
   - `proc_open()` with an array invokes `execvp()` directly, so `/bin/sh -c` is no longer part of the call chain and shell metacharacters lose their special meaning.

2. **Replaces the broken sanitizer with quoting for the smbclient mini-language.**
   - The old `_escapeShellCommand()` helper, which only escaped `;` and `\`, is removed.
   - A new `_quoteSmbArg()` helper escapes `\` and `"` inside the `smbclient -c` mini-language and rejects NUL/CR/LF bytes.
   - Every public method that builds a `smbclient -c` statement (`readFile`, `write`, `deleteFile`, `deleteFolder`, `rename`, `createFolder`, `listFolder`, `_createRoot`) now calls `_quoteSmbArg()` instead of concatenating double-quoted shell strings.

3. **Moves config-derived values into argv elements.**
   - `hostspec`, `share`, `username`, `port`, `ipaddress`, and `domain` are now passed as separate arguments (`-U`, `-p`, `-I`, `-W`) instead of being interpolated into a shell command line.
   - The `-U`/`%` anonymous-auth string replacement is now done by rewriting the argv array instead of `str_replace()` on a shell string.

## What the Fix Assumes

- **Assumption 1:** `proc_open()` with an array argument is implemented correctly on the target PHP runtime and never falls back to a shell parser.
- **Assumption 2:** The only untrusted data that reaches the `smbclient -c` command are filenames and path components; config parameters are still attacker-controlled in principle if the application lets a user supply them, but they are now argv elements and therefore not shell-interpreted.
- **Assumption 3:** `_quoteSmbArg()` is sufficient for the smbclient command parser: backslash and double-quote are the only characters that can break out of a `"..."` token, and NUL/CR/LF are rejected because they cannot be represented in SMB filenames.
- **Assumption 4:** No other code path in the SMB driver still assembles a shell command and invokes `proc_open()` with a string.

## What the Fix Covers

The patch explicitly updates every public method that can reach the vulnerable sink:

| Method | Vulnerable shell construction | Fixed argv construction |
|---|---|---|
| `readFile` | `get "<name>" <local>` | `get <_quoteSmbArg(name)> <_quoteSmbArg(local)>` |
| `write` | `put "<tmp>" "<name>"` | `put <_quoteSmbArg(tmp)> <_quoteSmbArg(name)>` |
| `deleteFile` | `del "<name>"` | `del <_quoteSmbArg(name)>` |
| `isFolder` | `quit` with `-D "<path>/<name>"` | `quit` with `-D <argv>` |
| `deleteFolder` | `rmdir "<name>"` | `rmdir <_quoteSmbArg(name)>` |
| `rename` | `rename "<old>" "<new>"` | `rename <_quoteSmbArg(old)> <_quoteSmbArg(new)>` |
| `createFolder` | `mkdir "<name>"` | `mkdir <_quoteSmbArg(name)>` |
| `listFolder` | `-D "<path>" -c "ls"` | `-D <argv> -c <argv>` |
| `_createRoot` | `mkdir "<dir>"` | `mkdir <_quoteSmbArg(dir)>` |

The regression test added in the same commit (`SmbCommandInjectionTest`) covers the same methods with `$(...)`, backtick, and newline payloads, plus a config-hostspec test.

## What the Fix Does NOT Cover (and Why It Is Still Complete for This Sink)

- **Other VFS drivers:** The repository also contains an `Ssh2.php` driver, but it uses `ssh2_exec()` with `escapeshellarg()` on the path and does not share the same `proc_open()` / `/bin/sh` sink. The reported CVE is specific to the SMB driver.
- **A full application entry point:** The proof targets the library API (`Horde_Vfs_Smb`) because the CVE describes an authenticated caller who can pass filenames through VFS operations. The fix is at the library layer, so any application entry point that reaches these methods is covered.
- **Config-level RCE via `smbclient` binary path:** The `smbclient` parameter is administrator-controlled; the fix does not need to sanitize it.
- **smbclient-side parsing bugs:** `_quoteSmbArg()` does not escape `;`, `|`, `&`, `$`, etc. because they are only meaningful to a shell. With `proc_open()` using an array, no shell is involved, so those characters are safely passed as literal filename bytes to `smbclient`.

## Threat Model / Security Policy

No `SECURITY.md`, threat-model document, or explicit security policy was found in the repository. The fix therefore operates under the standard library threat model implied by the CVE: an already-authenticated caller may supply arbitrary filenames and path components to `Horde_Vfs_Smb` methods, and the driver must not let those values reach a shell interpreter.

## Variant/Bypass Assessment

We tested the fixed commit against eight materially different entry points / data paths:

1. `createFolder` (filename payload) — the original repro path.
2. `deleteFile` (filename payload).
3. `writeData` (filename payload).
4. `rename` (new filename payload).
5. `readFile` (filename payload).
6. `deleteFolder` (folder-name payload).
7. `isFolder` (name payload, reaches `-D` argument).
8. `listFolder` (path payload, reaches `-D` argument).

On the vulnerable commit, all eight payloads executed `id > <marker>` before the fake `smbclient` was invoked. On the fixed commit, none of the markers were created and the fake `smbclient` received the literal payload inside the `smbclient -c` string. This confirms that the fix is not limited to `createFolder` and covers the same sink through every public method we identified.

No bypass of the v3.0.1 fix was found.