Cap open SFTP handles per session - #1135
Merged
Merged
Conversation
An authenticated peer could open SFTP file handles without limit, growing ssh->fileList until allocation failed and making the linear handle lookup progressively more expensive. - Limit tracked file handles per session to WOLFSSH_MAX_SFTP_HANDLES (default 64), bounding memory and the linear-lookup DoS surface. - Check the cap in SFTP_FileHandleCapped() from both RecvOpen branches after the path is resolved and before the file is opened. The open flags derived from the request may carry O_CREAT and O_TRUNC, so refusing after the open would create or truncate the target on a request the peer is told failed. SFTP_AddFileHandle() keeps the check as a backstop. - Refuse with FTP_FAILURE "Too Many Open File Handles" so the peer can tell a resource limit from a server malfunction. - Add a regress test via wolfSSH_SFTP_TestFileHandleCount() covering the cap, the refusal status reply, and that a refused open leaves the target file untouched.
Directory handles are tracked on ssh->dirList, a separate list from the file handles, so the SFTP_AddFileHandle cap does not reach them. A peer could loop on OPENDIR and grow that list until allocation failed. - Add SFTP_DirHandleCapped() and check it in both RecvOpenDir branches before the directory is opened or the name buffer allocated, so the rejection has nothing to unwind. Refusal sends an SFTP status like the existing permission-denied path rather than dropping the request. - Reuse WOLFSSH_MAX_SFTP_HANDLES, so each list is bounded separately. - Add wolfSSH_SFTP_TestDirHandleCount() and a regress test.
wolfSSL-Fenrir-bot
left a comment
wolfSSL-Fenrir-bot
left a comment
There was a problem hiding this comment.
Fenrir Automated Review — PR #1135
Scan targets checked: wolfssh-bugs, wolfssh-src
No new issues found in the changed files. ✅
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated no new comments.
Suppressed comments (1)
wolfssh/internal.h:900
- The macro comment says "0 yields a cap of one handle per list", but the preprocessor check below rejects any value < 1, so 0 will not compile. This is misleading documentation for anyone trying to tune the cap.
/* Maximum number of open handles tracked per session, applied separately to
* the file list and the directory list, so the worst case for one session is
* twice this value. Bounds memory use and keeps the linear handle lookup from
* becoming a CPU DoS vector. Must be at least 1; there is no "unlimited"
* setting, and 0 yields a cap of one handle per list. */
philljj
approved these changes
Jul 31, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Bounds the per-session SFTP handle lists, which an authenticated peer could previously grow without limit by looping on OPEN/OPENDIR until allocation failed. Also bounds the linear handle lookup those lists feed.
Supersedes #1017, which targeted SFTP_AddHandleNode/ssh->handleList, removed from master when the handle tracking moved to ssh->fileList. Thank you to @loganaden, for the bug report and the original fix.