Skip to content

Commit

Permalink
PR review: Fix potential memory leak when failing to parse a key of a…
Browse files Browse the repository at this point in the history 
…ny type.
  • Loading branch information
@ejohnstown
ejohnstown committed Nov 2, 2023
1 parent a65c411 commit 3b75aab
Showing 1 changed file with 91 additions and 45 deletions.
136 changes: 91 additions & 45 deletions src/ssh.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1487,12 +1487,13 @@ static int DoSshPubKey(const byte* in, word32 inSz, byte** out,
word32* outSz, const byte** outType, word32* outTypeSz,
void* heap)
{
int ret = WS_SUCCESS;
byte* newKey = NULL;
char* c;
char* last;
char* type = NULL;
char* key = NULL;
int ret = WS_SUCCESS;
word32 newKeySz, typeSz;

WOLFSSH_UNUSED(inSz);
WOLFSSH_UNUSED(heap);
Expand All @@ -1502,38 +1503,60 @@ static int DoSshPubKey(const byte* in, word32 inSz, byte** out,
type AAAABASE64ENCODEDKEYDATA comment
*/
c = WSTRDUP((const char*)in, heap, DYNTYPE_STRING);
type = WSTRTOK(c, " \n", &last);
key = WSTRTOK(NULL, " \n", &last);
if (c != NULL) {
type = WSTRTOK(c, " \n", &last);
key = WSTRTOK(NULL, " \n", &last);
}
else {
ret = WS_MEMORY_E;
}

if (type != NULL && key != NULL) {
const char* name;
word32 typeSz;
byte nameId;
if (ret == WS_SUCCESS) {
if (type == NULL || key == NULL) {
ret = WS_PARSE_E;
}
}

if (ret == WS_SUCCESS) {
typeSz = (word32)WSTRLEN(type);

nameId = NameToId(type, typeSz);
name = IdToName(nameId);
*outType = (const byte*)name;
*outTypeSz = typeSz;

/* set size based on sanity check in wolfSSL base64 decode
* function */
newKeySz = ((word32)WSTRLEN(key) * 3 + 3) / 4;
if (*out == NULL) {
/* set size based on sanity check in wolfSSL base64 decode
* function */
*outSz = ((word32)WSTRLEN(key) * 3 + 3) / 4;
newKey = (byte*)WMALLOC(*outSz, heap, DYNTYPE_PRIVKEY);
if (newKey == NULL) {
return WS_MEMORY_E;
ret = WS_MEMORY_E;
}
}
else {
if (*outSz < newKeySz) {
WLOG(WS_LOG_DEBUG, "PEM private key output size too small");
ret = WS_BUFFER_E;
}
else {
newKey = *out;
}
*out = newKey;
}

ret = Base64_Decode((byte*)key, (word32)WSTRLEN(key), *out, outSz);
}

if (ret != 0) {
WLOG(WS_LOG_DEBUG, "Base64 decode of public key failed.");
ret = WS_PARSE_E;
if (ret == WS_SUCCESS) {
ret = Base64_Decode((byte*)key, (word32)WSTRLEN(key),
newKey, &newKeySz);

if (ret == 0) {
*out = newKey;
*outSz = newKeySz;
*outType = (const byte *)IdToName(NameToId(type, typeSz));
*outTypeSz = (word32)WSTRLEN((const char*)*outType);
ret = WS_SUCCESS;
}
else {
WLOG(WS_LOG_DEBUG, "Base64 decode of public key failed.");
if (*out == NULL) {
WFREE(newKey, heap, DYNTYPE_PRIVKEY);
}
ret = WS_PARSE_E;
}
}

WFREE(c, heap, DYNTYPE_STRING);
Expand All @@ -1555,7 +1578,6 @@ static int DoAsn1Key(const byte* in, word32 inSz, byte** out,
if (newKey == NULL) {
return WS_MEMORY_E;
}
*out = newKey;
}
else {
if (*outSz < inSz) {
Expand All @@ -1564,15 +1586,23 @@ static int DoAsn1Key(const byte* in, word32 inSz, byte** out,
}
newKey = *out;
}
*outSz = inSz;
WMEMCPY(newKey, in, inSz);

ret = IdentifyAsn1Key(in, inSz, 1, heap);

if (ret > 0) {
*out = newKey;
*outSz = inSz;
WMEMCPY(newKey, in, inSz);
*outType = (const byte*)IdToName(ret);
*outTypeSz = (word32)WSTRLEN((const char*)*outType);
ret = WS_SUCCESS;
}
else {
WLOG(WS_LOG_DEBUG, "unable to identify key");
if (*out == NULL) {
WFREE(newKey, heap, DYNTYPE_PRIVKEY);
}
}

return ret;
}
Expand All @@ -1589,11 +1619,10 @@ static int DoPemKey(const byte* in, word32 inSz, byte** out,
WOLFSSH_UNUSED(heap);

if (*out == NULL) {
newKey = (byte*)WMALLOC(newKeySz, heap, DYNTYPE_PRIVKEY);
newKey = (byte*)WMALLOC(inSz, heap, DYNTYPE_PRIVKEY);
if (newKey == NULL) {
return WS_MEMORY_E;
}
*out = newKey;
}
else {
if (*outSz < inSz) {
Expand All @@ -1616,14 +1645,19 @@ static int DoPemKey(const byte* in, word32 inSz, byte** out,

if (ret == WS_SUCCESS) {
ret = IdentifyAsn1Key(newKey, newKeySz, 1, heap);
if (ret > 0) {
*outSz = newKeySz;
*outType = (const byte*)IdToName(ret);
*outTypeSz = (word32)WSTRLEN((const char*)*outType);
ret = WS_SUCCESS;
}
else {
WLOG(WS_LOG_DEBUG, "unable to identify key");
}

if (ret > 0) {
*out = newKey;
*outSz = newKeySz;
*outType = (const byte*)IdToName(ret);
*outTypeSz = (word32)WSTRLEN((const char*)*outType);
ret = WS_SUCCESS;
}
else {
WLOG(WS_LOG_DEBUG, "unable to identify key");
if (*out == NULL) {
WFREE(newKey, heap, DYNTYPE_PRIVKEY);
}
}

Expand All @@ -1644,7 +1678,6 @@ static int DoOpenSshKey(const byte* in, word32 inSz, byte** out,
if (newKey == NULL) {
return WS_MEMORY_E;
}
*out = newKey;
}
else {
if (*outSz < inSz) {
Expand All @@ -1659,15 +1692,28 @@ static int DoOpenSshKey(const byte* in, word32 inSz, byte** out,

ret = Base64_Decode((byte*)in, inSz, newKey, &newKeySz);
if (ret == 0) {
ret = WS_SUCCESS;
}
else {
WLOG(WS_LOG_DEBUG, "Base64 decode of public key failed.");
ret = WS_PARSE_E;
}

if (ret == WS_SUCCESS) {
ret = IdentifyOpenSshKey(newKey, newKeySz, heap);
if (ret > 0) {
*outSz = newKeySz;
*outType = (const byte*)IdToName(ret);
*outTypeSz = (word32)WSTRLEN((const char*)*outType);
ret = WS_SUCCESS;
}
else {
WLOG(WS_LOG_DEBUG, "unable to identify key");
}

if (ret > 0) {
*out = newKey;
*outSz = newKeySz;
*outType = (const byte*)IdToName(ret);
*outTypeSz = (word32)WSTRLEN((const char*)*outType);
ret = WS_SUCCESS;
}
else {
WLOG(WS_LOG_DEBUG, "unable to identify key");
if (*out == NULL) {
WFREE(newKey, heap, DYNTYPE_PRIVKEY);
}
}

Expand Down

0 comments on commit 3b75aab

Please sign in to comment.