From 487bcef2cc25db02e0d5a2e3f8c0101245338e03 Mon Sep 17 00:00:00 2001 From: John Safranek Date: Wed, 2 Sep 2020 12:11:24 -0700 Subject: [PATCH] Fix Memory 1. A couple places weren't passing the correct heap pointer to free. Normally this was ignored in the build because the WMALLOC macro left them out. Found using wolfCrypt memory logging. 2. Replaced the call to strdup() with wstrdup() that takes a heap and type parameter so sit may allocate a buffer with WMALLOC() and be freed correctly by WFREE(). 3. Tweaked the client to use a preallocated buffer for the private key rather than letting ReadKey allocate one. (Another WFREE() heap/type issue.) --- examples/client/client.c | 7 +++---- src/internal.c | 2 +- src/port.c | 15 +++++++++++++++ src/ssh.c | 2 +- wolfssh/port.h | 9 +++++---- 5 files changed, 25 insertions(+), 10 deletions(-) diff --git a/examples/client/client.c b/examples/client/client.c index c1e5752..bb7cb87 100644 --- a/examples/client/client.c +++ b/examples/client/client.c @@ -178,11 +178,12 @@ static void ShowUsage(void) static byte userPassword[256]; static byte userPublicKey[512]; static const byte* userPublicKeyType = NULL; -static byte* userPrivateKey = NULL; /* Will be allocated by Read Key. */ +static byte userPrivateKeyBuf[1191]; /* Size equal to hanselPrivateRsaSz. */ +static byte* userPrivateKey = userPrivateKeyBuf; static const byte* userPrivateKeyType = NULL; static word32 userPublicKeySz = 0; static word32 userPublicKeyTypeSz = 0; -static word32 userPrivateKeySz = 0; +static word32 userPrivateKeySz = sizeof(userPrivateKeyBuf); static word32 userPrivateKeyTypeSz = 0; static byte isPrivate = 0; @@ -1083,8 +1084,6 @@ THREAD_RETURN WOLFSSH_THREAD client_test(void* args) WCLOSESOCKET(sockFd); wolfSSH_free(ssh); wolfSSH_CTX_free(ctx); - if (userPrivateKey != NULL) - free(userPrivateKey); if (ret != WS_SUCCESS) err_sys("Closing client stream failed. Connection could have been closed by peer"); diff --git a/src/internal.c b/src/internal.c index 6dac8a7..a3563f6 100644 --- a/src/internal.c +++ b/src/internal.c @@ -1720,7 +1720,7 @@ int GetStringAlloc(void* heap, char** s, byte* buf, word32 len, word32 *idx) str[strSz] = '\0'; if (*s != NULL) - WFREE(*s, ssh->ctx->heap, DYNTYPE_STRING); + WFREE(*s, heap, DYNTYPE_STRING); *s = str; } diff --git a/src/port.c b/src/port.c index 0909b36..f9714cd 100644 --- a/src/port.c +++ b/src/port.c @@ -428,6 +428,21 @@ int WS_DeleteFileA(const char* fileName, void* heap) #ifndef WSTRING_USER +char* wstrdup(const char* s1, void* heap, int type) +{ + char* s2 = NULL; + + if (s1 != NULL) { + unsigned int sz; + sz = (unsigned int)WSTRLEN(s1) + 1; + s2 = (char*)WMALLOC(sz, heap, type); + if (s2 != NULL) + WSTRNCPY(s2, (const char*)s1, sz); + } + return s2; +} + + char* wstrnstr(const char* s1, const char* s2, unsigned int n) { unsigned int s2_len = (unsigned int)WSTRLEN(s2); diff --git a/src/ssh.c b/src/ssh.c index 985d24a..4f42337 100644 --- a/src/ssh.c +++ b/src/ssh.c @@ -1403,7 +1403,7 @@ int wolfSSH_ReadKey_buffer(const byte* in, word32 inSz, int format, SSH format is: type AAAABASE64ENCODEDKEYDATA comment */ - c = WSTRDUP((const char*)in, heap); + c = WSTRDUP((const char*)in, heap, DYNTYPE_STRING); type = WSTRTOK(c, " \n", &last); key = WSTRTOK(NULL, " \n", &last); diff --git a/wolfssh/port.h b/wolfssh/port.h index bb3d799..3395a98 100644 --- a/wolfssh/port.h +++ b/wolfssh/port.h @@ -308,7 +308,7 @@ extern "C" { #define WSNPRINTF(s,n,f,...) _snprintf_s((s),(n),(n),(f),##__VA_ARGS__) #define WVSNPRINTF(s,n,f,...) _vsnprintf_s((s),(n),(n),(f),##__VA_ARGS__) #define WSTRTOK(s1,s2,s3) strtok_s((s1),(s2),(s3)) - #define WSTRDUP(s,h) _strdup((s)) + #define WSTRDUP(s,h,t) _strdup((s)) #elif defined(MICROCHIP_MPLAB_HARMONY) || defined(MICROCHIP_PIC32) #include #define WSTRNCPY(s1,s2,n) strncpy((s1),(s2),(n)) @@ -316,7 +316,7 @@ extern "C" { #define WSNPRINTF(s,n,f,...) snprintf((s),(n),(f),##__VA_ARGS__) #define WVSNPRINTF(s,n,f,...) vsnprintf((s),(n),(f),##__VA_ARGS__) #define WSTRTOK(s1,s2,s3) strtok_r((s1),(s2),(s3)) - #define WSTRDUP(s,h) strdup((s)) + #define WSTRDUP(s,h,t) strdup((s)) #elif defined(RENESAS_CSPLUS) #include #define WSTRNCPY(s1,s2,n) strncpy((s1),(s2),(n)) @@ -324,7 +324,7 @@ extern "C" { #define WSNPRINTF(s,n,f,...) snprintf((s),(n),(f),__VA_ARGS__) #define WVSNPRINTF(s,n,f,...) vsnprintf((s),(n),(f),__VA_ARGS__) #define WSTRTOK(s1,s2,s3) strtok_r((s1),(s2),(s3)) - #define WSTRDUP(s,h) strdup((s)) + #define WSTRDUP(s,h,t) strdup((s)) #else #ifndef FREESCALE_MQX #include @@ -334,7 +334,8 @@ extern "C" { #define WSNPRINTF(s,n,f,...) snprintf((s),(n),(f),##__VA_ARGS__) #define WVSNPRINTF(s,n,f,...) vsnprintf((s),(n),(f),##__VA_ARGS__) #define WSTRTOK(s1,s2,s3) strtok_r((s1),(s2),(s3)) - #define WSTRDUP(s,h) strdup((s)) + WOLFSSL_API char* wstrdup(const char*, void*, int); + #define WSTRDUP(s,h,t) wstrdup((s),(h),(t)) #endif #endif /* WSTRING_USER */