From fc0e339467078709bf616cb7662bee53ff45ba29 Mon Sep 17 00:00:00 2001 From: Mats Klepsland Date: Wed, 28 Mar 2018 22:25:46 +0200 Subject: [PATCH] app-layer-ssl: fix use-after-free (CID 14336229) Nullify JA3 buffer on free to avoid use-after-free vulnerability. --- src/app-layer-ssl.c | 12 ++++++------ src/util-ja3.c | 22 ++++++++++++---------- src/util-ja3.h | 2 +- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/app-layer-ssl.c b/src/app-layer-ssl.c index 54ba32534d..c89bd369e9 100644 --- a/src/app-layer-ssl.c +++ b/src/app-layer-ssl.c @@ -650,7 +650,7 @@ static inline int TLSDecodeHSHelloCipherSuites(SSLState *ssl_state, while (processed_len < cipher_suites_length) { if (!(HAS_SPACE(2))) { - Ja3BufferFree(ja3_cipher_suites); + Ja3BufferFree(&ja3_cipher_suites); goto invalid_length; } @@ -660,7 +660,7 @@ static inline int TLSDecodeHSHelloCipherSuites(SSLState *ssl_state, if (TLSDecodeValueIsGREASE(cipher_suite) != 1) { rc = Ja3BufferAddValue(ja3_cipher_suites, cipher_suite); if (rc != 0) { - Ja3BufferFree(ja3_cipher_suites); + Ja3BufferFree(&ja3_cipher_suites); return -1; } } @@ -1034,11 +1034,11 @@ invalid_length: error: if (ja3_extensions != NULL) - Ja3BufferFree(ja3_extensions); + Ja3BufferFree(&ja3_extensions); if (ja3_elliptic_curves != NULL) - Ja3BufferFree(ja3_elliptic_curves); + Ja3BufferFree(&ja3_elliptic_curves); if (ja3_elliptic_curves_pf != NULL) - Ja3BufferFree(ja3_elliptic_curves_pf); + Ja3BufferFree(&ja3_elliptic_curves_pf); return -1; } @@ -2299,7 +2299,7 @@ static void SSLStateFree(void *p) SCFree(ssl_state->server_connp.sni); if (ssl_state->ja3_str) - Ja3BufferFree(ssl_state->ja3_str); + Ja3BufferFree(&ssl_state->ja3_str); if (ssl_state->ja3_hash) SCFree(ssl_state->ja3_hash); diff --git a/src/util-ja3.c b/src/util-ja3.c index a676d0223b..7898468fb0 100644 --- a/src/util-ja3.c +++ b/src/util-ja3.c @@ -50,15 +50,17 @@ JA3Buffer *Ja3BufferInit(void) * * \param buffer The buffer to free. */ -void Ja3BufferFree(JA3Buffer *buffer) +void Ja3BufferFree(JA3Buffer **buffer) { - DEBUG_VALIDATE_BUG_ON(buffer == NULL); + DEBUG_VALIDATE_BUG_ON(*buffer == NULL); - if (buffer->data != NULL) { - SCFree(buffer->data); + if ((*buffer)->data != NULL) { + SCFree((*buffer)->data); + (*buffer)->data = NULL; } - SCFree(buffer); + SCFree(*buffer); + *buffer = NULL; } /** @@ -123,8 +125,8 @@ int Ja3BufferAppendBuffer(JA3Buffer *buffer1, JA3Buffer *buffer2) int rc = Ja3BufferResizeIfFull(buffer1, buffer2->used); if (rc != 0) { - Ja3BufferFree(buffer1); - Ja3BufferFree(buffer2); + Ja3BufferFree(&buffer1); + Ja3BufferFree(&buffer2); return -1; } @@ -136,7 +138,7 @@ int Ja3BufferAppendBuffer(JA3Buffer *buffer1, JA3Buffer *buffer2) buffer1->used, ",%s", buffer2->data); } - Ja3BufferFree(buffer2); + Ja3BufferFree(&buffer2); return 0; } @@ -179,7 +181,7 @@ int Ja3BufferAddValue(JA3Buffer *buffer, uint32_t value) if (buffer->data == NULL) { SCLogError(SC_ERR_MEM_ALLOC, "Error allocating memory for JA3 data"); - Ja3BufferFree(buffer); + Ja3BufferFree(&buffer); return -1; } buffer->size = JA3_BUFFER_INITIAL_SIZE; @@ -189,7 +191,7 @@ int Ja3BufferAddValue(JA3Buffer *buffer, uint32_t value) int rc = Ja3BufferResizeIfFull(buffer, value_len); if (rc != 0) { - Ja3BufferFree(buffer); + Ja3BufferFree(&buffer); return -1; } diff --git a/src/util-ja3.h b/src/util-ja3.h index 10a2255692..e82d198884 100644 --- a/src/util-ja3.h +++ b/src/util-ja3.h @@ -33,7 +33,7 @@ typedef struct JA3Buffer_ { } JA3Buffer; JA3Buffer *Ja3BufferInit(void); -void Ja3BufferFree(JA3Buffer *); +void Ja3BufferFree(JA3Buffer **); int Ja3BufferAppendBuffer(JA3Buffer *, JA3Buffer *); int Ja3BufferAddValue(JA3Buffer *, uint32_t); char *Ja3GenerateHash(JA3Buffer *);