Cap open SFTP directory handles per session

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.

src/wolfsftp.c

@@ -2773,6 +2773,22 @@ struct WS_DIR_LIST {
 };
 
 
+/* returns 1 when the session already holds the maximum directory handles */
+static int SFTP_DirHandleCapped(WOLFSSH* ssh)
+{
+    WS_DIR_LIST* cur;
+    word32 count = 0;
+
+    for (cur = ssh->dirList; cur != NULL; cur = cur->next) {
+        if (++count >= WOLFSSH_MAX_SFTP_HANDLES) {
+            return 1;
+        }
+    }
+
+    return 0;
+}
+
+
 /* Handles packet to open a directory
  *
  * returns WS_SUCCESS on success
@@ -2792,6 +2808,7 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
     byte*  out = NULL;
     byte idFlat[WOLFSSH_HANDLE_ID_SZ];
     char per[] = "Permission denied";
+    char tooMany[] = "Too Many Open Directory Handles";
 
     if (ssh == NULL) {
         return WS_BAD_ARGUMENT;
@@ -2819,6 +2836,13 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
         return WS_BUFFER_E;
     }
 
+    /* check the cap before opening, so there is nothing to unwind */
+    if (SFTP_DirHandleCapped(ssh)) {
+        WLOG(WS_LOG_SFTP, "Too many open directory handles for session");
+        rc = SFTP_SendStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, tooMany);
+        return (rc == WS_SUCCESS) ? WS_BAD_FILE_E : rc;
+    }
+
     if (WOPENDIR(ssh->fs, ssh->ctx->heap, &ctx, dir) != 0) {
         WLOG(WS_LOG_SFTP, "Error with opening directory: %s", dir);
         if (wolfSSH_SFTP_CreateStatus(ssh, WOLFSSH_FTP_NOFILE, reqId,
@@ -2905,6 +2929,7 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
     char name[MAX_PATH];
     char clean[WOLFSSH_MAX_FILENAME];
     char per[] = "Permission denied";
+    char tooMany[] = "Too Many Open Directory Handles";
 
     if (ssh == NULL) {
         return WS_BAD_ARGUMENT;
@@ -2943,6 +2968,13 @@ int wolfSSH_SFTP_RecvOpenDir(WOLFSSH* ssh, int reqId, byte* data, word32 maxSz)
         return WS_BUFFER_E;
     }
 
+    /* check the cap before allocating, so there is nothing to unwind */
+    if (SFTP_DirHandleCapped(ssh)) {
+        WLOG(WS_LOG_SFTP, "Too many open directory handles for session");
+        rc = SFTP_SendStatus(ssh, WOLFSSH_FTP_FAILURE, reqId, tooMany);
+        return (rc == WS_SUCCESS) ? WS_BAD_FILE_E : rc;
+    }
+
     /* plus one to make sure is null terminated */
     dirNameSz = (word32)WSTRLEN(clean) + 1;
     dirName = (char*)WMALLOC(dirNameSz, ssh->ctx->heap, DYNTYPE_BUFFER);
@@ -6360,6 +6392,23 @@ int wolfSSH_SFTP_TestFileHandleCount(WOLFSSH* ssh)
     return count;
 }
 
+#ifndef NO_WOLFSSH_DIR
+/* Return the number of open directory handles tracked for the session. */
+int wolfSSH_SFTP_TestDirHandleCount(WOLFSSH* ssh)
+{
+    WS_DIR_LIST* cur;
+    int count = 0;
+
+    if (ssh == NULL) {
+        return 0;
+    }
+    for (cur = ssh->dirList; cur != NULL; cur = cur->next) {
+        count++;
+    }
+    return count;
+}
+#endif /* NO_WOLFSSH_DIR */
+
 /* Close the underlying descriptor of the head tracked file handle out of band,
  * leaving the node in the list with a now-stale fd. The next RecvClose on that
  * handle will see its close() fail, exercising the path that must still drop

tests/regress.c

@@ -3263,6 +3263,98 @@ static void TestSftpHandleLimit(void)
     wolfSSH_CTX_free(ctx);
 }
 
+#ifndef NO_WOLFSSH_DIR
+/* Directory handles live on ssh->dirList, a separate list from the file
+ * handles, so the SFTP_AddFileHandle cap does not apply to them. Without its
+ * own limit a peer could loop on OPENDIR and grow that list without bound.
+ * Same shape as TestSftpHandleLimit: fill to the cap, confirm the next OPENDIR
+ * is refused, close one and confirm a fresh OPENDIR succeeds again. */
+static void TestSftpDirHandleLimit(void)
+{
+    WOLFSSH_CTX* ctx;
+    WOLFSSH* ssh;
+    int rid = 400;
+    int reqId;
+    int i;
+    word32 idx;
+    word32 replySz;
+    const byte* reply;
+    const word32 hOff = WOLFSSH_SFTP_HEADER + UINT32_SZ; /* handle in reply */
+    byte handles[WOLFSSH_MAX_SFTP_HANDLES][WOLFSSH_HANDLE_ID_SZ];
+    byte pkt[256];
+    char cwd[WOLFSSH_MAX_FILENAME];
+
+    ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL);
+    AssertNotNull(ctx);
+    ssh = wolfSSH_new(ctx);
+    AssertNotNull(ssh);
+    AssertIntEQ(wolfSSH_SFTP_TestRecvStateInit(ssh), WS_SUCCESS);
+
+    WMEMSET(cwd, 0, sizeof(cwd));
+    AssertNotNull(WGETCWD(ssh->fs, cwd, sizeof(cwd) - 1));
+    AssertIntEQ(wolfSSH_SFTP_SetDefaultPath(ssh, cwd), WS_SUCCESS);
+
+    /* open the cap's worth of handles on "."; all must succeed */
+    for (i = 0; i < WOLFSSH_MAX_SFTP_HANDLES; i++) {
+        idx = 0;
+        SftpPutU32(1, pkt + idx); idx += UINT32_SZ;
+        pkt[idx++] = '.';
+        AssertIntEQ(wolfSSH_SFTP_RecvOpenDir(ssh, rid++, pkt, idx), WS_SUCCESS);
+        reply = wolfSSH_SFTP_TestRecvReply(ssh, &replySz);
+        AssertNotNull(reply);
+        AssertTrue(replySz >= hOff + WOLFSSH_HANDLE_ID_SZ);
+        WMEMCPY(handles[i], reply + hOff, WOLFSSH_HANDLE_ID_SZ);
+    }
+    AssertIntEQ(wolfSSH_SFTP_TestDirHandleCount(ssh),
+            WOLFSSH_MAX_SFTP_HANDLES);
+
+    /* one past the cap must be refused, and must not grow the list */
+    idx = 0;
+    SftpPutU32(1, pkt + idx); idx += UINT32_SZ;
+    pkt[idx++] = '.';
+    reqId = rid++;
+    AssertTrue(wolfSSH_SFTP_RecvOpenDir(ssh, reqId, pkt, idx) != WS_SUCCESS);
+    AssertIntEQ(wolfSSH_SFTP_TestDirHandleCount(ssh),
+            WOLFSSH_MAX_SFTP_HANDLES);
+    /* the peer must be told, and told it was a failure rather than silence */
+    AssertSftpStatusReply(ssh, reqId, WOLFSSH_FTP_FAILURE);
+
+    /* free one slot; a fresh OPENDIR must now succeed again */
+    idx = 0;
+    SftpPutU32(WOLFSSH_HANDLE_ID_SZ, pkt + idx); idx += UINT32_SZ;
+    WMEMCPY(pkt + idx, handles[0], WOLFSSH_HANDLE_ID_SZ);
+    idx += WOLFSSH_HANDLE_ID_SZ;
+    AssertIntEQ(wolfSSH_SFTP_RecvClose(ssh, rid++, pkt, idx), WS_SUCCESS);
+    AssertIntEQ(wolfSSH_SFTP_TestDirHandleCount(ssh),
+            WOLFSSH_MAX_SFTP_HANDLES - 1);
+
+    idx = 0;
+    SftpPutU32(1, pkt + idx); idx += UINT32_SZ;
+    pkt[idx++] = '.';
+    AssertIntEQ(wolfSSH_SFTP_RecvOpenDir(ssh, rid++, pkt, idx), WS_SUCCESS);
+    reply = wolfSSH_SFTP_TestRecvReply(ssh, &replySz);
+    AssertNotNull(reply);
+    AssertTrue(replySz >= hOff + WOLFSSH_HANDLE_ID_SZ);
+    WMEMCPY(handles[0], reply + hOff, WOLFSSH_HANDLE_ID_SZ);
+    AssertIntEQ(wolfSSH_SFTP_TestDirHandleCount(ssh),
+            WOLFSSH_MAX_SFTP_HANDLES);
+
+    /* close every handle and clean up */
+    for (i = 0; i < WOLFSSH_MAX_SFTP_HANDLES; i++) {
+        idx = 0;
+        SftpPutU32(WOLFSSH_HANDLE_ID_SZ, pkt + idx); idx += UINT32_SZ;
+        WMEMCPY(pkt + idx, handles[i], WOLFSSH_HANDLE_ID_SZ);
+        idx += WOLFSSH_HANDLE_ID_SZ;
+        AssertIntEQ(wolfSSH_SFTP_RecvClose(ssh, rid++, pkt, idx), WS_SUCCESS);
+    }
+    AssertIntEQ(wolfSSH_SFTP_TestDirHandleCount(ssh), 0);
+
+    wolfSSH_SFTP_TestRecvStateFree(ssh);
+    wolfSSH_free(ssh);
+    wolfSSH_CTX_free(ctx);
+}
+#endif /* NO_WOLFSSH_DIR */
+
 /* A failed close() must still drop the handle from the session tracking list;
  * otherwise the stale descriptor lingers and is closed a second time when the
  * session is torn down. Open a file, invalidate its descriptor out of band so
@@ -5783,6 +5875,10 @@ int main(int argc, char** argv)
     #endif
     /* open file handles are capped per session */
     TestSftpHandleLimit();
+    #ifndef NO_WOLFSSH_DIR
+    /* open directory handles are capped per session */
+    TestSftpDirHandleLimit();
+    #endif
     /* a failed close still drops the handle from the tracking list */
     TestSftpCloseFailureRemovesHandle();
     #endif

wolfssh/internal.h

@@ -896,8 +896,11 @@ WOLFSSH_LOCAL int wolfSSH_GetPath(const char* defaultPath, byte* in,
 #ifdef WOLFSSH_SFTP
 #define WOLFSSH_MAX_SFTPOFST 3
 
-/* Maximum number of open file handles tracked per session. Bounds memory use
- * and keeps the linear handle lookup from becoming a CPU DoS vector. */
+/* 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. */
 #ifndef WOLFSSH_MAX_SFTP_HANDLES
     #define WOLFSSH_MAX_SFTP_HANDLES 64
 #endif

wolfssh/wolfsftp.h

@@ -324,6 +324,9 @@ WOLFSSH_LOCAL void wolfSSH_SFTP_ShowSizes(void);
                 word32* sz);
         WOLFSSH_API void wolfSSH_SFTP_TestRecvStateFree(WOLFSSH* ssh);
         WOLFSSH_API int wolfSSH_SFTP_TestFileHandleCount(WOLFSSH* ssh);
+        #ifndef NO_WOLFSSH_DIR
+            WOLFSSH_API int wolfSSH_SFTP_TestDirHandleCount(WOLFSSH* ssh);
+        #endif
         WOLFSSH_API int wolfSSH_SFTP_TestInvalidateHeadFd(WOLFSSH* ssh);
     #endif
     #if defined(WOLFSSL_NUCLEUS) && !defined(NO_WOLFSSH_MKTIME)