fix: SHELL_Subsystem returns without restoring raised privileges

[MarkAtwood]

Bug

SHELL_Subsystem() (apps/wolfsshd/wolfsshd.c) temporarily raises privileges via
wolfSSHD_AuthRaisePermissions(conn->auth) to look up user information. Four
error paths between that raise and the normal privilege drops return
WS_FATAL_ERROR without restoring privileges:

• pipe(stdoutPipe) failure
• pipe(stderrPipe) failure
• pipe(stdinPipe) failure
• forkpty() failure

An earlier revision of this description said the caller discards the return
value. That is wrong -- HandleConnection() does capture it
(ret = SHELL_Subsystem(...)). What actually happens is worse: it falls out of
the session switch and runs its whole teardown -- wolfSSH_shutdown(), up to
ten wolfSSH_worker() iterations with 100 ms sleeps, wolfSSH_free(), then a
shutdown()/recv() drain loop on the socket. That is real protocol processing
on peer-supplied packets, performed as root.

This only has effect when UsePrivilegeSeparation is yes or sandbox
(otherwise the raise is a no-op), and reaching it requires fd/process
exhaustion, so severity is low -- but the raise should always be paired with a
drop.

Fix

Restore privileges with wolfSSHD_AuthReducePermissions(conn->auth) before each
of the four early returns, matching the existing exit(1)-on-drop-failure
handling used on the normal child/parent paths.

wolfSSHD_AuthReducePermissions() is the right call here rather than
wolfSSHD_AuthReducePermissionsUser(): it restores the sshd account via
setegid/seteuid and is a no-op unless privilege separation is on, which
matches the severity note above. exit(1) on drop failure is safe -- on POSIX
every connection is a forked child, so it takes down only that connection's
process; on Windows the function body is inside #ifndef _WIN32 and always
returns WS_SUCCESS, so the branch never fires.

Two further items from review are included:

1. The forkpty() path leaked six pipe descriptors. When
   !ptyReq || forcedCmd, all three pipes are already open and none of the six
   descriptors were closed before the return. The three pipe() failure paths
   do clean up what they opened; this one did not. Since the failure mode this
   change exists to handle is fd/process exhaustion, leaking fds on the way out
   is the wrong direction. They are now closed and the slots reset to -1,
   keeping the invariant the child branch already follows that an entry is
   either a live descriptor or -1.

2. Log wording. The new lines use the daemon's existing phrasing for this
   event, "[SSHD] Error lowering permissions level", rather than introducing a
   third spelling alongside that and "[SSHD] Error setting user ID".

Verification

• Built --enable-all (incl. wolfsshd) against wolfSSL master on macOS and on
  Ubuntu; compiles clean, no new warnings.
• make check: 10 PASS / 1 SKIP / 0 FAIL.
• wolfsshd suite on Ubuntu: sshd_exec_test.sh, sshd_term_size_test.sh,
  sshd_large_sftp_test.sh, sshd_bad_sftp_test.sh, sshd_scp_fail.sh and
  sshd_term_close_test.sh PASSED, ssh_kex_algos.sh SKIPPED, host key
  ownership gate PASSED.

sshd_term_close_test.sh is intermittent in that environment. Measured over six
interleaved iterations per tree on an idle host, unpatched master scored
2 pass / 4 fail and this branch 3 pass / 3 fail, so the flakiness is
pre-existing and not introduced here.

The four new error paths are not covered by any test. The existing
fault-injection harness (apps/wolfsshd/test/sshd_privdrop_fail_test.sh plus
sshd_privdrop_preload.c) cannot reach them: the interposer overrides
setregid/setreuid while wolfSSHD_AuthReducePermissions() uses
setegid/seteuid, and the test config sets UsePrivilegeSeparation no, which
makes both the raise and the reduce no-ops. Extending it would need pipe(),
setegid() and seteuid() overrides plus a sandbox case, and is not done
here.

Reported by static analysis (Fenrir finding F-6980).

[Copilot]

Pull request overview

Fixes a privilege-separation correctness issue in SHELL_Subsystem() (wolfsshd) where several early error returns could leave the per-connection handler running with temporarily raised privileges.

Changes:

• Drop raised privileges via wolfSSHD_AuthReducePermissions(conn->auth) before returning on pipe() failures (stdout/stderr/stdin).
• Drop raised privileges via wolfSSHD_AuthReducePermissions(conn->auth) before returning on forkpty() failure.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.