WolfSSL
/
wolfssh
mirror of https://github.com/wolfSSL/wolfssh.git
1
0
Fork 0
Code Issues Releases Wiki Activity

Fix double-free crash and socket-close spin

pull/855/head
Andrew Hutchings 2025-12-01 15:41:13 +00:00 committed by John Safranek
parent 5ae5c250e2
commit dee1c59f26
5 changed files with 137 additions and 29 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

35
apps/wolfssh/common.c
View File

@ -45,10 +45,12 @@ static byte* userPublicKey = userPublicKeyBuf;
static const byte* userPublicKeyType = NULL;
static byte userPassword[256];
static const byte* userPrivateKeyType = NULL;
static byte userPublicKeyAlloc = 0;
static word32 userPublicKeySz = 0;
static byte pubKeyLoaded = 0; /* was a public key loaded */
static byte userPrivateKeyBuf[1191];
static byte* userPrivateKey = userPrivateKeyBuf;
static byte userPrivateKeyAlloc = 0;
static word32 userPublicKeyTypeSz = 0;
static word32 userPrivateKeySz = sizeof(userPrivateKeyBuf);
static word32 userPrivateKeyTypeSz = 0;
@ -670,6 +672,13 @@ int ClientUseCert(const char* certName)
userPublicKeyType = publicKeyType;
userPublicKeyTypeSz = (word32)WSTRLEN((const char*)publicKeyType);
pubKeyLoaded = 1;
userPublicKeyAlloc = 1;
}
else {
userPublicKey = userPublicKeyBuf;
userPublicKeySz = 0;
userPublicKeyType = NULL;
userPublicKeyAlloc = 0;
}
#else
fprintf(stderr, "Certificate support not compiled in");
@ -687,12 +696,22 @@ int ClientSetPrivateKey(const char* privKeyName)
{
int ret;
userPrivateKeyAlloc = 0;
userPrivateKey = NULL; /* create new buffer based on parsed input */
ret = wolfSSH_ReadKey_file(privKeyName,
(byte**)&userPrivateKey, &userPrivateKeySz,
(const byte**)&userPrivateKeyType, &userPrivateKeyTypeSz,
&isPrivate, NULL);
if (ret == 0) {
userPrivateKeyAlloc = 1;
}
else {
userPrivateKey = userPrivateKeyBuf;
userPrivateKeySz = sizeof(userPrivateKeyBuf);
userPrivateKeyType = NULL;
}
return ret;
}
@ -703,6 +722,7 @@ int ClientUsePubKey(const char* pubKeyName)
{
int ret;
userPublicKeyAlloc = 0;
userPublicKey = NULL; /* create new buffer based on parsed input */
ret = wolfSSH_ReadKey_file(pubKeyName,
&userPublicKey, &userPublicKeySz,
@ -711,6 +731,11 @@ int ClientUsePubKey(const char* pubKeyName)
if (ret == 0) {
pubKeyLoaded = 1;
userPublicKeyAlloc = 1;
}
else {
userPublicKey = userPublicKeyBuf;
userPublicKeySz = 0;
}
return ret;
@ -747,11 +772,17 @@ int ClientLoadCA(WOLFSSH_CTX* ctx, const char* caCert)
void ClientFreeBuffers(void)
{
if (userPublicKey != userPublicKeyBuf) {
if (userPublicKeyAlloc && userPublicKey != NULL) {
WFREE(userPublicKey, NULL, DYNTYPE_PRIVKEY);
userPublicKey = userPublicKeyBuf;
userPublicKeySz = 0;
userPublicKeyAlloc = 0;
}
if (userPrivateKey != userPrivateKeyBuf) {
if (userPrivateKeyAlloc && userPrivateKey != NULL) {
WFREE(userPrivateKey, NULL, DYNTYPE_PRIVKEY);
userPrivateKey = userPrivateKeyBuf;
userPrivateKeySz = sizeof(userPrivateKeyBuf);
userPrivateKeyAlloc = 0;
}
}

79
apps/wolfssh/wolfssh.c
View File

@ -220,6 +220,7 @@ typedef struct thread_args {
wolfSSL_Mutex lock;
byte rawMode;
byte quit;
int readError;
} thread_args;
#ifdef _POSIX_THREADS
@ -390,6 +391,7 @@ static THREAD_RET readPeer(void* in)
int bufSz = sizeof(buf);
thread_args* args = (thread_args*)in;
int ret = 0;
int stop = 0;
int fd = wolfSSH_get_fd(args->ssh);
word32 bytes;
#ifdef USE_WINDOWS_API
@ -398,11 +400,6 @@ static THREAD_RET readPeer(void* in)
fd_set readSet;
fd_set errSet;
FD_ZERO(&readSet);
FD_ZERO(&errSet);
FD_SET(fd, &readSet);
FD_SET(fd, &errSet);
#ifdef USE_WINDOWS_API
if (args->rawMode == 0) {
DWORD wrd;
@ -431,9 +428,13 @@ static THREAD_RET readPeer(void* in)
#endif
while (ret >= 0) {
#if defined(WOLFSSH_TERM) && defined(USE_WINDOWS_API)
#if defined(WOLFSSH_TERM) && defined(USE_WINDOWS_API)
(void)windowMonitor(args);
#endif
#endif
FD_ZERO(&readSet);
FD_ZERO(&errSet);
FD_SET(fd, &readSet);
FD_SET(fd, &errSet);
bytes = select(fd + 1, &readSet, NULL, &errSet, NULL);
wc_LockMutex(&args->lock);
@ -458,18 +459,18 @@ static THREAD_RET readPeer(void* in)
} while (ret > 0);
}
else if (ret <= 0) {
if (ret == WS_FATAL_ERROR) {
ret = wolfSSH_get_error(args->ssh);
if (ret == WS_WANT_READ) {
continue;
}
#ifdef WOLFSSH_AGENT
else if (ret == WS_CHAN_RXD) {
byte agentBuf[512];
int rxd, txd;
word32 channel = 0;
int err = (ret == WS_FATAL_ERROR) ?
wolfSSH_get_error(args->ssh) : ret;
if (err == WS_WANT_READ) {
bytes = 0;
}
#ifdef WOLFSSH_AGENT
else if (err == WS_CHAN_RXD) {
byte agentBuf[512];
int rxd, txd;
word32 channel = 0;
wolfSSH_GetLastRxId(args->ssh, &channel);
wolfSSH_GetLastRxId(args->ssh, &channel);
rxd = wolfSSH_ChannelIdRead(args->ssh, channel,
agentBuf, sizeof(agentBuf));
if (rxd > 4) {
@ -495,9 +496,17 @@ static THREAD_RET readPeer(void* in)
WMEMSET(agentBuf, 0, sizeof(agentBuf));
continue;
}
#endif /* WOLFSSH_AGENT */
#endif /* WOLFSSH_AGENT */
else if (err == WS_CBIO_ERR_CONN_CLOSE ||
err == WS_SOCKET_ERROR_E ||
err == WS_MSGID_NOT_ALLOWED_E) {
args->readError = err;
ret = err;
stop = 1;
bytes = 0;
}
else if (ret != WS_EOF) {
else if (err != WS_EOF) {
wc_UnLockMutex(&args->lock);
err_sys("Stream read failed.");
}
}
@ -517,12 +526,16 @@ static THREAD_RET readPeer(void* in)
}
#endif
}
ret = wolfSSH_stream_peek(args->ssh, buf, bufSz);
if (ret <= 0) {
bytes = 0; /* read it all */
if (!stop) {
ret = wolfSSH_stream_peek(args->ssh, buf, bufSz);
if (ret <= 0) {
bytes = 0; /* read it all */
}
}
}
wc_UnLockMutex(&args->lock);
if (stop)
break;
}
#if !defined(WOLFSSH_NO_ECC) && defined(FP_ECC) && defined(HAVE_THREAD_LS)
wc_ecc_fp_free(); /* free per thread cache */
@ -791,7 +804,8 @@ static int config_parse_command_line(struct config* config,
if (found != NULL) {
*found = '\0';
if (config->user) {
free(config->user);
WFREE(config->user, NULL, 0);
config->user = NULL;
}
sz = WSTRLEN(cursor);
config->user = (char*)WMALLOC(sz + 1, NULL, 0);
@ -818,7 +832,7 @@ static int config_parse_command_line(struct config* config,
strcpy(config->hostname, cursor);
}
free(dest);
WFREE(dest, NULL, 0);
myoptind++;
}
@ -874,18 +888,23 @@ static int config_cleanup(struct config* config)
{
if (config->user) {
WFREE(config->user, NULL, 0);
config->user = NULL;
}
if (config->hostname) {
WFREE(config->hostname, NULL, 0);
config->hostname = NULL;
}
if (config->keyFile) {
WFREE(config->keyFile, NULL, 0);
config->keyFile = NULL;
}
if (config->pubKeyFile) {
WFREE(config->pubKeyFile, NULL, 0);
config->pubKeyFile = NULL;
}
if (config->command) {
WFREE(config->command, NULL, 0);
config->command = NULL;
}
return 0;
@ -900,6 +919,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
SOCKADDR_IN_T clientAddr;
socklen_t clientAddrSz = sizeof(clientAddr);
int ret = 0;
int ioErr = 0;
byte keepOpen = 1;
#ifdef USE_WINDOWS_API
byte rawMode = 0;
@ -1037,6 +1057,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
wc_InitMutex(&arg.lock);
arg.ssh = ssh;
arg.readError = 0;
#ifdef WOLFSSH_TERM
arg.quit = 0;
#if (defined(__OSX__) || defined(__APPLE__))
@ -1082,12 +1103,14 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
sem_destroy(&windowSem);
#endif
#endif /* WOLFSSH_TERM */
ioErr = arg.readError;
#elif defined(_MSC_VER)
thread_args arg;
HANDLE thread[2];
arg.ssh = ssh;
arg.rawMode = rawMode;
arg.readError = 0;
wc_InitMutex(&arg.lock);
if (config.command) {
@ -1107,6 +1130,7 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
WaitForSingleObject(thread[1], INFINITE);
CloseHandle(thread[0]);
CloseHandle(thread[1]);
ioErr = arg.readError;
#else
err_sys("No threading to use");
#endif
@ -1139,10 +1163,13 @@ static THREAD_RETURN WOLFSSH_THREAD wolfSSH_Client(void* args)
#if defined(WOLFSSH_TERM) || defined(WOLFSSH_SHELL)
((func_args*)args)->return_code = wolfSSH_GetExitStatus(ssh);
#endif
if (ioErr != 0 && ((func_args*)args)->return_code == 0) {
((func_args*)args)->return_code = 1;
}
wolfSSH_free(ssh);
wolfSSH_CTX_free(ctx);
if (ret != WS_SUCCESS && ret != WS_SOCKET_ERROR_E) {
if ((ret != WS_SUCCESS && ret != WS_SOCKET_ERROR_E) || ioErr != 0) {
WLOG(WS_LOG_DEBUG, "Closing client stream failed");
#if defined(WOLFSSH_TERM) || defined(WOLFSSH_SHELL)
/* override return value, do not want to return success if connection

2
src/internal.c
View File

@ -691,6 +691,8 @@ INLINE static int IsMessageAllowedClient(WOLFSSH *ssh, byte msg)
/* Error if expecting a specific message and didn't receive. */
if (ssh->handshake && ssh->handshake->expectMsgId != MSGID_NONE) {
/* The explicit expectMsgId check supersedes the old
* IsMessageAllowedKeying() stub for rekey filtering. */
if (msg != ssh->handshake->expectMsgId) {
WLOG(WS_LOG_DEBUG,
"Message ID %u not the expected message %u",

3
tests/include.am
View File

@ -45,7 +45,8 @@ tests_kex_test_CPPFLAGS = -DNO_MAIN_DRIVER $(AM_CPPFLAGS)
tests_kex_test_LDADD = src/libwolfssh.la
tests_kex_test_DEPENDENCIES = src/libwolfssh.la
tests_regress_test_SOURCES = tests/regress.c
tests_regress_test_SOURCES = tests/regress.c apps/wolfssh/common.c \
apps/wolfssh/common.h
tests_regress_test_CPPFLAGS = -DNO_MAIN_DRIVER -DWOLFSSH_TEST_INTERNAL $(AM_CPPFLAGS)
tests_regress_test_LDADD = src/libwolfssh_test.la
tests_regress_test_DEPENDENCIES = src/libwolfssh_test.la

47
tests/regress.c
View File

@ -19,10 +19,13 @@
#include <stdlib.h>
#include <arpa/inet.h>
#include <string.h>
#include <unistd.h>
#include <fcntl.h>
#include <wolfssh/port.h>
#include <wolfssh/ssh.h>
#include <wolfssh/internal.h>
#include "apps/wolfssh/common.h"
#ifndef WOLFSSH_NO_ABORT
#define WABORT() abort()
@ -362,6 +365,44 @@ static void TestKexInitRejectedWhenKeying(WOLFSSH* ssh)
AssertFalse(allowed);
}
/* Ensure client buffer cleanup tolerates multiple invocations after allocs. */
static void TestClientBuffersIdempotent(void)
{
int ret;
ret = ClientUsePubKey("keys/gretel-key-rsa.pub");
AssertIntEQ(ret, 0);
ret = ClientSetPrivateKey("keys/gretel-key-rsa.pem");
AssertIntEQ(ret, 0);
ClientFreeBuffers();
/* Should be safe to call again without double free. */
ClientFreeBuffers();
}
/* Simulate Ctrl+D (stdin EOF) during password prompt; expect failure but no crash. */
static void TestPasswordEofNoCrash(void)
{
WS_UserAuthData auth;
int savedStdin, devNull, ret;
WMEMSET(&auth, 0, sizeof(auth));
savedStdin = dup(STDIN_FILENO);
devNull = open("/dev/null", O_RDONLY);
AssertTrue(devNull >= 0);
AssertTrue(dup2(devNull, STDIN_FILENO) >= 0);
ret = ClientUserAuth(WOLFSSH_USERAUTH_PASSWORD, &auth, NULL);
AssertIntEQ(ret, WOLFSSH_USERAUTH_FAILURE);
close(devNull);
dup2(savedStdin, STDIN_FILENO);
close(savedStdin);
ClientFreeBuffers();
}
int main(int argc, char** argv)
{
@ -390,6 +431,12 @@ int main(int argc, char** argv)
TestChannelBlockedBeforeAuth(ssh);
TestChannelAllowedAfterAuth(ssh);
TestKexInitRejectedWhenKeying(ssh);
TestClientBuffersIdempotent();
TestPasswordEofNoCrash();
/* TODO: add app-level regressions that simulate stdin EOF/password
* prompts and mid-session socket closes once the test harness can
* drive the wolfssh client without real sockets/tty. */
ResetSession(ssh);
wolfSSH_free(ssh);