Repository navigation
SCP: receive through a per-session dirfd - #1306
ejohnstown wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
🟡 Changes recommended
Base-path and timestamp races can still escape confinement, while fallback validation and CI rebuilding are incomplete.
4 open findings
What changed in this PR
Moves SCP receive state from the process working directory into per-session paths and POSIX directory descriptors.
Changes:
- Adds dirfd-relative file and directory creation with symlink protections.
- Tracks and releases receive paths, descriptors, and parent identities per session.
- Adds regression tests and fallback-build CI coverage.
| File | Description |
|---|---|
src/wolfscp.c |
Implements per-session receive traversal. |
src/internal.c |
Initializes and frees receive state. |
wolfssh/internal.h |
Adds session receive fields. |
wolfssh/port.h |
Adds dirfd portability macros. |
configure.ac |
Detects required *at() APIs. |
tests/unit.c |
Tests traversal, isolation, and moves. |
tests/api.c |
Updates recursive-test assumptions. |
scripts/scp.test |
Tests symlink write refusal. |
.github/workflows/scp-test.yml |
Adds fallback configuration coverage. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1306
Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 3 of 6 in-scope changed file(s) opened by the reviewer; not opened: src/internal.c, tests/api.c, wolfssh/internal.h
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
7d10d6c to
832f954
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1306
Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 3 of 4 in-scope changed file(s) opened by the reviewer; not opened: src/port.c
Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)
This review was generated automatically by Fenrir. Reported findings require changes before merge.
Review tier: Lite
The default SCP receive callback keeps its destination per session, the base path plus each directory entered, in place of the process working directory that every session of a threaded server shares. With openat() it holds that directory open, reaches it and everything created below it with O_NOFOLLOW, and confirms a reopened parent is the one entered. - port.h gains WOPENAT, WMKDIRAT, WOLFSSH_O_SEARCH with a search check as chdir() made, and WOLFSSH_HAVE_DIRFD, which sizes WOLFSSH so it keys only on configure's openat() probe; WOLFSSH_NO_DIRFD opts out - ports without them open by the full path after a wIsDirNoFollow() screen of the base and of each directory entered; a refused base frees the receive path - the receive path is capped at DEFAULT_SCP_FILE_NAME_SZ like the send side's; futimes() stands in for futimens() on hosts with openat() but not futimens(), such as macOS before 10.13 - unit: ScpRecvCallback_SessionPath walks down and back with the working directory untouched, then has the tree moved out from under it; a linked base and a file-named directory are refused - scp.test: a planted symlink at the destination name keeps its target; scp-test.yml also builds and tests the fallback Issue: F-14758, F-13998
832f954 to
8907c72
Compare
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1306
Scan targets checked: wolfssh-src, wolfssh-bugs
Coverage: 1 of 1 in-scope changed file(s) opened by the reviewer
Fenrir result: Approved ✅
No new issues found in the changed files.
Advisory only — this automated result does not count as a GitHub approval.
Review tier: Lite
Fenrir's latest completed scan found no issues; clearing the prior automated change request.
The default SCP receive callback keeps its destination per session instead of changing the process working directory, and on POSIX walks it through a directory descriptor with O_NOFOLLOW opens.