Bind SCP file timestamps to open descriptor

[yosuke-wolfssl]

Bind SCP file timestamps to the open descriptor (fix symlink TOCTOU)

Summary

In the SCP receive path, peer-supplied modification/access times were applied with a path-based utimes() after the destination file was closed. On POSIX, utimes() follows symlinks, so a local process running as the authenticated user could replace the just-written file with a symlink in the window between the close and the timestamp update, redirecting the peer-controlled mTime/aTime onto an arbitrary target inode.

This change binds the timestamp update to the open file descriptor where possible, and uses a symlink-safe path call where it isn't, so the update can no longer be redirected to an arbitrary target.

Approach — a priority ladder

The timestamp update now prefers the strongest mechanism the platform supports:

1. futimens() on the open fd, before close (POSIX, HAVE_FUTIMENS) — bound to the inode opened in NEW_FILE; no path lookup, so a path swap is irrelevant. Fully closes the race.
2. _futime() on the open CRT fd, before close (Windows) — same idea; no longer re-opens by path.
3. utimensat(AT_FDCWD, path, ts, AT_SYMLINK_NOFOLLOW) by path, after close (POSIX without futimens, HAVE_UTIMENSAT) — no descriptor to bind to, but AT_SYMLINK_NOFOLLOW refuses to follow a swapped-in symlink, defeating the exploited redirect-to-arbitrary-target.
4. utimes() by path, after close — last resort only when neither of the above is detected.

futimens keeps strict priority: WOLFSSH_SCP_FD_UTIMES is keyed on the fd-based macro only, so utimensat is reached solely on builds without futimens.

Changes

src/wolfscp.c

• SetTimestampInfo() now takes the open WFILE* and implements the ladder above (fd → path-no-follow → path).
• Buffered data is flushed before applying the timestamps, so the subsequent fclose() cannot write and overwrite the modification time.
• The WOLFSSH_SCP_FILE_DONE handler applies the timestamp before WFCLOSE on descriptor-capable platforms. If timestamps are present but no file was opened, it aborts rather than falling back to an unsafe path update.
• Windows branch null-checks fp, matching the POSIX contract.
• libc return values are normalized to WS_FATAL_ERROR on failure, consistent with the documented WS_SUCCESS/negative contract.

wolfssh/port.h

• Added WFUTIMES(fd, t) (→ futimens, gated on HAVE_FUTIMENS) — the fd-bound primary.
• Added WUTIMES_NOFOLLOW(f, t) (→ utimensat(..., AT_SYMLINK_NOFOLLOW), gated on HAVE_UTIMENSAT) — the symlink-safe path fallback.
• Both wrappers convert the SCP struct timeval pair to the struct timespec pair the new calls expect, and include <time.h> so struct timespec is guaranteed visible (it is not required to come transitively from <sys/stat.h>/<sys/time.h>).
• The two macros are gated independently, so the fallback stays maximally safe wherever utimensat exists; priority is enforced at the call site.

configure.ac

• Replaced the plain AC_CHECK_FUNCS entry with header-aware AC_LINK_IFELSE probes that define HAVE_FUTIMENS / HAVE_UTIMENSAT. Both functions are POSIX.1-2008 and their declarations are feature-test-macro gated, so a link-only test can report the libc symbol present while the prototype stays hidden at compile time — silently disabling the hardening. Probing through the real header (and prototype) makes a positive result build-safe.

tests/unit.c

• test_ScpRecvCallback_Timestamp — drives the default receive callback through a full single-file receive and asserts the written file's mtime/atime equal the peer-supplied values. On descriptor builds it also asserts a FILE_DONE carrying timestamps with no open file aborts (no path fallback) and pins the flush-before-set ordering.
• test_ScpTimestamp_NoFollow (HAVE_UTIMENSAT && WOLFSSH_HAVE_SYMLINK) — sets times through a symlink and asserts the time lands on the symlink itself while the target it points to is left untouched.

Platform behavior

Platform	Timestamp update
POSIX with futimens	futimens() on the open fd, before close
Windows	_futime() on the open CRT fd, before close
POSIX without futimens, with utimensat	utimensat(AT_SYMLINK_NOFOLLOW) by path, after close
POSIX without either	utimes() by path, after close (residual race)
Nucleus / other path-only ports	path-based WUTIMES (unchanged)
VxWorks / Zephyr / Microchip	no-op (unchanged)

Known limitation

futimens fully closes the race. The utimensat fallback defeats the symlink-to-arbitrary-target redirect (the exploited case) but a narrow window remains where the path could be swapped for a regular file before the call. The plain-utimes last resort retains the original behavior and is only reachable on builds that detect neither futimens nor utimensat; autotools builds get one of the two.

Testing

• tests/unit.test — ScpRecvCallback_Timestamp and ScpTimestamp_NoFollow pass; full suite green under ASan + UBSan (no leak detection on macOS).
• Negative controls: removing the pre-flush makes ScpRecvCallback_Timestamp fail (fclose clobbers mtime); flipping AT_SYMLINK_NOFOLLOW→0 makes ScpTimestamp_NoFollow fail (follows the symlink); restoring the fix makes both pass.
• Detection: configure reports checking for futimens... yes / checking for utimensat... yes via the header-aware probes; HAVE_FUTIMENS/HAVE_UTIMENSAT defined in config.h.
• Production build (--enable-scp --enable-sftp, no sanitizers) — clean, no new warnings.

[Copilot]

Pull request overview

Hardens the SCP receive path against a symlink TOCTOU by applying peer-supplied timestamps to the still-open destination file descriptor (when supported), instead of applying them later via a path-based utimes().

Changes:

• Update SCP receive timestamp application to prefer fd-based timestamp setting (futimes / _futime) and flush before applying times to prevent close-time writes from clobbering mtime.
• Add WFUTIMES port macro gated by HAVE_FUTIMES, and detect futimes in configure.ac.
• Add an end-to-end unit test asserting received files end up with the peer-supplied mtime/atime.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File	Description
src/wolfscp.c	Applies timestamps via the open file descriptor (when available) with pre-flush ordering to prevent TOCTOU and mtime clobbering.
wolfssh/port.h	Introduces WFUTIMES macro gated by HAVE_FUTIMES for portability.
configure.ac	Adds futimes feature detection to enable fd-based timestamp updates.
tests/unit.c	Adds an end-to-end regression test for SCP receive timestamp behavior.

---

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

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1037

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

[Copilot]

Pull request overview

Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #1037

Scan targets checked: wolfssh-bugs, wolfssh-src

No new issues found in the changed files. ✅

[yosuke-wolfssl]

Hi @ejohnstown ,
I fixed the issues. Please review this again