From c3e98a7179705e38154997899a6723e8294437ef Mon Sep 17 00:00:00 2001 From: aidan garske Date: Mon, 13 Jul 2026 15:50:39 -0700 Subject: [PATCH] Allocate channelName dynamically to shrink WOLFSSH struct by ~4KB --- src/internal.c | 28 ++++++++----- src/ssh.c | 40 ++++++++++++++++--- tests/api.c | 97 ++++++++++++++++++++++++++++++++++++++++++++++ wolfssh/internal.h | 2 +- 4 files changed, 152 insertions(+), 15 deletions(-) diff --git a/src/internal.c b/src/internal.c index 8b7fd28cc..4f751ef89 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1565,6 +1565,11 @@ void SshResourceFree(WOLFSSH* ssh, void* heap) if (ssh->userName) { WFREE(ssh->userName, heap, DYNTYPE_STRING); } + if (ssh->channelName != NULL) { + WFREE(ssh->channelName, heap, DYNTYPE_STRING); + ssh->channelName = NULL; + ssh->channelNameSz = 0; + } if (ssh->peerProtoId) { WFREE(ssh->peerProtoId, heap, DYNTYPE_STRING); } @@ -19015,6 +19020,9 @@ int SendChannelRequest(WOLFSSH* ssh, byte* name, word32 nameSz) if (ssh == NULL) ret = WS_BAD_ARGUMENT; + if (ret == WS_SUCCESS && nameSz > 0 && name == NULL) + ret = WS_BAD_ARGUMENT; + if (ret == WS_SUCCESS) { channel = ChannelFind(ssh, ssh->defaultPeerChannelId, WS_CHANNEL_ID_PEER); @@ -19079,17 +19087,19 @@ int SendChannelRequest(WOLFSSH* ssh, byte* name, word32 nameSz) #ifdef DEBUG_WOLFSSH /* only compile in code for checks on type if in debug mode */ - switch (ssh->connectChannelId) { - case WOLFSSH_SESSION_EXEC: - WLOG(WS_LOG_INFO, " command = %s", name); - break; + if (name != NULL) { + switch (ssh->connectChannelId) { + case WOLFSSH_SESSION_EXEC: + WLOG(WS_LOG_INFO, " command = %.*s", (int)nameSz, name); + break; - case WOLFSSH_SESSION_SUBSYSTEM: - WLOG(WS_LOG_INFO, " subsystem = %s", name); - break; + case WOLFSSH_SESSION_SUBSYSTEM: + WLOG(WS_LOG_INFO, " subsystem = %.*s", (int)nameSz, name); + break; - default: - break; + default: + break; + } } #endif diff --git a/src/ssh.c b/src/ssh.c index f73754e92..9c44e8fa0 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1614,6 +1614,12 @@ int wolfSSH_SetChannelType(WOLFSSH* ssh, byte type, byte* name, word32 nameSz) switch (type) { case WOLFSSH_SESSION_SHELL: + /* shell has no name; drop any name left by a prior subsystem/exec */ + if (ssh->channelName != NULL) { + WFREE(ssh->channelName, ssh->ctx->heap, DYNTYPE_STRING); + ssh->channelName = NULL; + } + ssh->channelNameSz = 0; ssh->connectChannelId = type; break; @@ -1624,21 +1630,45 @@ int wolfSSH_SetChannelType(WOLFSSH* ssh, byte type, byte* name, word32 nameSz) } FALL_THROUGH; - case WOLFSSH_SESSION_SUBSYSTEM: - ssh->connectChannelId = type; - if (name != NULL && nameSz < WOLFSSH_MAX_CHN_NAMESZ) { - WMEMCPY(ssh->channelName, name, nameSz); - ssh->channelNameSz = nameSz; + case WOLFSSH_SESSION_SUBSYSTEM: { + byte* newName; + + if (name != NULL && nameSz > 0 && nameSz < WOLFSSH_MAX_CHN_NAMESZ) { + /* only (re)allocate when the name changed; SFTP/SCP retry + * loops re-set the same name on every poll */ + if (ssh->channelName == NULL || ssh->channelNameSz != nameSz || + WMEMCMP(ssh->channelName, name, nameSz) != 0) { + newName = (byte*)WMALLOC(nameSz + 1, ssh->ctx->heap, + DYNTYPE_STRING); + if (newName == NULL) { + return WS_MEMORY_E; + } + WMEMCPY(newName, name, nameSz); + newName[nameSz] = 0; + if (ssh->channelName != NULL) { + WFREE(ssh->channelName, ssh->ctx->heap, DYNTYPE_STRING); + } + ssh->channelName = newName; + ssh->channelNameSz = nameSz; + } } else { + /* invalid name ignored; type set but WS_SUCCESS returned */ WLOG(WS_LOG_DEBUG, "No subsystem name or name was too large"); } + ssh->connectChannelId = type; break; + } #ifdef WOLFSSH_TERM case WOLFSSH_SESSION_TERMINAL: /* send a pseudo-terminal request and shell channel */ ssh->sendTerminalRequest = 1; + if (ssh->channelName != NULL) { + WFREE(ssh->channelName, ssh->ctx->heap, DYNTYPE_STRING); + ssh->channelName = NULL; + } + ssh->channelNameSz = 0; ssh->connectChannelId = WOLFSSH_SESSION_SHELL; break; #endif diff --git a/tests/api.c b/tests/api.c index 54959cbd3..b4709cf23 100644 --- a/tests/api.c +++ b/tests/api.c @@ -285,6 +285,102 @@ static void test_wolfSSH_SetUsername(void) } +static void test_wolfSSH_SetChannelType(void) +{ +#ifndef NO_WOLFSSH_CLIENT + WOLFSSH_CTX* ctx; + WOLFSSH* ssh; + const byte sub1[] = "sftp"; + const byte sub2[] = "a-longer-subsystem-name"; + byte* prevName; + + AssertIntNE(WS_SUCCESS, wolfSSH_SetChannelType(NULL, + WOLFSSH_SESSION_SHELL, NULL, 0)); + + AssertNotNull(ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL)); + AssertNotNull(ssh = wolfSSH_new(ctx)); + + AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_SHELL, NULL, 0)); + AssertNull(ssh->channelName); + AssertIntEQ(0, ssh->channelNameSz); + + AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_SUBSYSTEM, NULL, 0)); + AssertNull(ssh->channelName); + + AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1, WOLFSSH_MAX_CHN_NAMESZ)); + AssertNull(ssh->channelName); + + AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1, + (word32)(sizeof(sub1) - 1))); + AssertNotNull(ssh->channelName); + AssertIntEQ((int)(sizeof(sub1) - 1), (int)ssh->channelNameSz); + AssertIntEQ(0, strcmp((const char*)ssh->channelName, (const char*)sub1)); + AssertIntEQ(0, ssh->channelName[ssh->channelNameSz]); /* NUL terminated */ + + /* re-setting the same name must not reallocate (no churn) */ + prevName = ssh->channelName; + AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1, + (word32)(sizeof(sub1) - 1))); + AssertIntEQ(1, ssh->channelName == prevName); + + /* a rejected (oversize) name must leave the previous buffer intact */ + AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1, WOLFSSH_MAX_CHN_NAMESZ)); + AssertIntEQ(1, ssh->channelName == prevName); + AssertIntEQ((int)(sizeof(sub1) - 1), (int)ssh->channelNameSz); + AssertIntEQ(0, strcmp((const char*)ssh->channelName, (const char*)sub1)); + + /* a zero-length name is ignored and preserves the stored name */ + AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub1, 0)); + AssertIntEQ(1, ssh->channelName == prevName); + AssertIntEQ((int)(sizeof(sub1) - 1), (int)ssh->channelNameSz); + + /* repeated set frees the previous buffer before replacing it */ + AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_SUBSYSTEM, (byte*)sub2, + (word32)(sizeof(sub2) - 1))); + AssertNotNull(ssh->channelName); + AssertIntEQ((int)(sizeof(sub2) - 1), (int)ssh->channelNameSz); + AssertIntEQ(0, strcmp((const char*)ssh->channelName, (const char*)sub2)); + + /* EXEC reaches the same alloc path via fallthrough on the client side */ + AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_EXEC, (byte*)sub1, + (word32)(sizeof(sub1) - 1))); + AssertNotNull(ssh->channelName); + AssertIntEQ((int)(sizeof(sub1) - 1), (int)ssh->channelNameSz); + AssertIntEQ(0, strcmp((const char*)ssh->channelName, (const char*)sub1)); + + /* switching to SHELL frees and clears a prior subsystem/exec name */ + AssertIntEQ(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_SHELL, NULL, 0)); + AssertNull(ssh->channelName); + AssertIntEQ(0, ssh->channelNameSz); + + /* unknown channel type is rejected */ + AssertIntNE(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_UNKNOWN, NULL, 0)); + + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); + + /* server-side EXEC is rejected */ + AssertNotNull(ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_SERVER, NULL)); + AssertNotNull(ssh = wolfSSH_new(ctx)); + AssertIntNE(WS_SUCCESS, wolfSSH_SetChannelType(ssh, + WOLFSSH_SESSION_EXEC, (byte*)sub1, (word32)(sizeof(sub1) - 1))); + wolfSSH_free(ssh); + wolfSSH_CTX_free(ctx); +#endif /* NO_WOLFSSH_CLIENT */ +} + + enum WS_TestFormatTypes { TEST_GOOD_FORMAT_ASN1 = WOLFSSH_FORMAT_ASN1, TEST_GOOD_FORMAT_PEM = WOLFSSH_FORMAT_PEM, @@ -3837,6 +3933,7 @@ int wolfSSH_ApiTest(int argc, char** argv) test_client_wolfSSH_new(); test_wolfSSH_set_fd(); test_wolfSSH_SetUsername(); + test_wolfSSH_SetChannelType(); test_wolfSSH_ConvertConsole(); test_wolfSSH_CTX_UsePrivateKey_buffer(); test_wolfSSH_CTX_UseCert_buffer(); diff --git a/wolfssh/internal.h b/wolfssh/internal.h index a37e88514..5d5122d0b 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -915,7 +915,7 @@ struct WOLFSSH { word32 channelListSz; word32 defaultPeerChannelId; word32 connectChannelId; - byte channelName[WOLFSSH_MAX_CHN_NAMESZ]; + byte* channelName; word32 channelNameSz; word32 lastRxId;