diff --git a/src/internal.c b/src/internal.c index f142c0db9..05dc408f0 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1967,11 +1967,7 @@ 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; - } + FreeChannelName(ssh, heap); if (ssh->peerProtoId) { WFREE(ssh->peerProtoId, heap, DYNTYPE_STRING); } @@ -4470,6 +4466,20 @@ static void FreeChannelCommand(void* heap, char* command, word32 commandSz) } +/* Wipe and free the client's stored exec/subsystem name. */ +void FreeChannelName(WOLFSSH* ssh, void* heap) +{ + WOLFSSH_UNUSED(heap); + + if (ssh->channelName != NULL) { + WS_FORCEZERO(ssh->channelName, ssh->channelNameSz); + WFREE(ssh->channelName, heap, DYNTYPE_STRING); + ssh->channelName = NULL; + } + ssh->channelNameSz = 0; +} + + void ChannelDelete(WOLFSSH_CHANNEL* channel, void* heap) { WOLFSSH_UNUSED(heap); @@ -13866,6 +13876,7 @@ static int DoChannelRequest(WOLFSSH* ssh, else WLOG(WS_LOG_DEBUG, " %s = %s", name, value); } + WS_FORCEZERO(value, sizeof(value)); } else if (ChannelRequestIs(type, typeSz, "shell")) { ret = DoChannelRequestSession(ssh, channelId, channel, diff --git a/src/ssh.c b/src/ssh.c index 09bf297fe..6459aa79d 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -2059,11 +2059,7 @@ 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; + FreeChannelName(ssh, ssh->ctx->heap); ssh->connectChannelId = type; break; @@ -2100,9 +2096,7 @@ int wolfSSH_SetChannelType(WOLFSSH* ssh, byte type, byte* name, word32 nameSz) } WMEMCPY(newName, name, nameSz); newName[nameSz] = 0; - if (ssh->channelName != NULL) { - WFREE(ssh->channelName, ssh->ctx->heap, DYNTYPE_STRING); - } + FreeChannelName(ssh, ssh->ctx->heap); ssh->channelName = newName; ssh->channelNameSz = nameSz; } @@ -2128,11 +2122,7 @@ int wolfSSH_SetChannelType(WOLFSSH* ssh, byte type, byte* name, word32 nameSz) 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; + FreeChannelName(ssh, ssh->ctx->heap); ssh->connectChannelId = WOLFSSH_SESSION_SHELL; break; #endif diff --git a/tests/unit.c b/tests/unit.c index ffc3517d6..65619890f 100644 --- a/tests/unit.c +++ b/tests/unit.c @@ -20355,6 +20355,140 @@ static int test_ChannelDelete_zeroesCommand(void) return result; } + +/* Store a command line as the client's exec channel name and return the + * buffer holding it, or NULL. */ +static const byte* SetExecName(WOLFSSH* ssh, const char* command) +{ + if (wolfSSH_SetChannelType(ssh, WOLFSSH_SESSION_EXEC, (byte*)command, + (word32)WSTRLEN(command)) != WS_SUCCESS) + return NULL; + return ssh->channelName; +} + +static int IsWiped(const byte* p, word32 sz) +{ + word32 i; + + if (p == NULL || !IsRetained((void*)p)) + return 0; + for (i = 0; i < sz; i++) { + if (p[i] != 0) + return 0; + } + return 1; +} + +/* Verify every release of the client's exec command line in + * ssh->channelName wipes it first: switching to shell or terminal, + * replacing it with another command, and wolfSSH_free(). The + * retain-on-free allocator is installed just around each release so the + * freed bytes can be read back. */ +static int test_ChannelName_zeroedOnFree(void) +{ + static const char command[] = "sh -c 'login --password hunter2'"; + WOLFSSH_CTX* ctx = NULL; + WOLFSSH* ssh = NULL; + const byte* name; + word32 commandSz = (word32)WSTRLEN(command); + int result = 0; + wolfSSL_Malloc_cb prevMf = NULL; + wolfSSL_Free_cb prevFf = NULL; + wolfSSL_Realloc_cb prevRf = NULL; + + ctx = wolfSSH_CTX_new(WOLFSSH_ENDPOINT_CLIENT, NULL); + if (ctx == NULL) + return -740; + ssh = wolfSSH_new(ctx); + if (ssh == NULL) { + result = -741; + goto out; + } + wolfSSL_GetAllocators(&prevMf, &prevFf, &prevRf); + + /* exec to shell */ + if ((name = SetExecName(ssh, command)) == NULL) { + result = -742; + goto out; + } + if (wolfSSL_SetAllocators(RetainMalloc, RetainFree, + RetainRealloc) != 0) { + result = -743; + goto out; + } + wolfSSH_SetChannelType(ssh, WOLFSSH_SESSION_SHELL, NULL, 0); + wolfSSL_SetAllocators(prevMf, prevFf, prevRf); + if (!IsWiped(name, commandSz)) { + result = -744; + goto out; + } + DrainRetained(); + + /* exec replaced by another exec */ + if ((name = SetExecName(ssh, command)) == NULL) { + result = -745; + goto out; + } + if (wolfSSL_SetAllocators(RetainMalloc, RetainFree, + RetainRealloc) != 0) { + result = -746; + goto out; + } + wolfSSH_SetChannelType(ssh, WOLFSSH_SESSION_EXEC, (byte*)"ls", 2); + wolfSSL_SetAllocators(prevMf, prevFf, prevRf); + if (!IsWiped(name, commandSz)) { + result = -747; + goto out; + } + DrainRetained(); + +#ifdef WOLFSSH_TERM + /* exec to terminal */ + if ((name = SetExecName(ssh, command)) == NULL) { + result = -748; + goto out; + } + if (wolfSSL_SetAllocators(RetainMalloc, RetainFree, + RetainRealloc) != 0) { + result = -749; + goto out; + } + wolfSSH_SetChannelType(ssh, WOLFSSH_SESSION_TERMINAL, NULL, 0); + wolfSSL_SetAllocators(prevMf, prevFf, prevRf); + if (!IsWiped(name, commandSz)) { + result = -750; + goto out; + } + DrainRetained(); +#endif + + /* wolfSSH_free() */ + if ((name = SetExecName(ssh, command)) == NULL) { + result = -751; + goto out; + } + if (wolfSSL_SetAllocators(RetainMalloc, RetainFree, + RetainRealloc) != 0) { + result = -752; + goto out; + } + wolfSSH_free(ssh); + wolfSSL_SetAllocators(prevMf, prevFf, prevRf); + ssh = NULL; + if (!IsWiped(name, commandSz)) { + result = -753; + goto out; + } + +out: + DrainRetained(); + if (ssh != NULL) + wolfSSH_free(ssh); + if (ctx != NULL) + wolfSSH_CTX_free(ctx); + return result; +} + #ifdef WOLFSSH_TEST_INTERNAL /* [uint32 channelId][string "exec"][byte wantReply][string command] */ @@ -26945,6 +27079,11 @@ int wolfSSH_UnitTest(int argc, char** argv) (unitResult == 0 ? "SUCCESS" : "FAILED")); testResult = testResult || unitResult; + unitResult = test_ChannelName_zeroedOnFree(); + printf("ChannelName_zeroedOnFree: %s\n", + (unitResult == 0 ? "SUCCESS" : "FAILED")); + testResult = testResult || unitResult; + #ifdef WOLFSSH_TEST_INTERNAL unitResult = test_DoChannelRequest_zeroesRefusedCommand(); printf("DoChannelRequest_zeroesRefusedCommand: %s\n", diff --git a/wolfssh/internal.h b/wolfssh/internal.h index d9c550995..0ba16f481 100644 --- a/wolfssh/internal.h +++ b/wolfssh/internal.h @@ -1571,6 +1571,7 @@ WOLFSSH_LOCAL WOLFSSH_CTX* CtxInit(WOLFSSH_CTX* ctx, byte side, void* heap); WOLFSSH_LOCAL void CtxResourceFree(WOLFSSH_CTX* ctx); WOLFSSH_LOCAL WOLFSSH* SshInit(WOLFSSH* ssh, WOLFSSH_CTX* ctx); WOLFSSH_LOCAL void SshResourceFree(WOLFSSH* ssh, void* heap); +WOLFSSH_LOCAL void FreeChannelName(WOLFSSH* ssh, void* heap); WOLFSSH_LOCAL WOLFSSH_CHANNEL* ChannelNew(WOLFSSH* ssh, byte channelType, word32 initialWindowSz, word32 maxPacketSz);