From 598ef96b7b1e9ae9e778fcbdea3d9d0fb99ad67c Mon Sep 17 00:00:00 2001 From: Mats Klepsland Date: Thu, 29 Mar 2018 17:57:42 +0200 Subject: [PATCH] app-layer-ssl: really fix CID 1433623 --- src/app-layer-ssl.c | 20 +++++++-------- src/util-ja3.c | 61 ++++++++++++++++++++++++--------------------- src/util-ja3.h | 4 +-- 3 files changed, 44 insertions(+), 41 deletions(-) diff --git a/src/app-layer-ssl.c b/src/app-layer-ssl.c index 0f290e93d3..df39555de3 100644 --- a/src/app-layer-ssl.c +++ b/src/app-layer-ssl.c @@ -563,7 +563,7 @@ static inline int TLSDecodeHSHelloVersion(SSLState *ssl_state, if (ssl_state->ja3_str == NULL) return -1; - int rc = Ja3BufferAddValue(ssl_state->ja3_str, version); + int rc = Ja3BufferAddValue(&ssl_state->ja3_str, version); if (rc != 0) return -1; } @@ -658,7 +658,7 @@ static inline int TLSDecodeHSHelloCipherSuites(SSLState *ssl_state, input += 2; if (TLSDecodeValueIsGREASE(cipher_suite) != 1) { - rc = Ja3BufferAddValue(ja3_cipher_suites, cipher_suite); + rc = Ja3BufferAddValue(&ja3_cipher_suites, cipher_suite); if (rc != 0) { return -1; } @@ -667,7 +667,7 @@ static inline int TLSDecodeHSHelloCipherSuites(SSLState *ssl_state, processed_len += 2; } - rc = Ja3BufferAppendBuffer(ssl_state->ja3_str, ja3_cipher_suites); + rc = Ja3BufferAppendBuffer(&ssl_state->ja3_str, &ja3_cipher_suites); if (rc == -1) { return -1; } @@ -817,7 +817,7 @@ static inline int TLSDecodeHSHelloExtensionEllipticCurves(SSLState *ssl_state, input += 2; if (TLSDecodeValueIsGREASE(elliptic_curve) != 1) { - int rc = Ja3BufferAddValue(ja3_elliptic_curves, + int rc = Ja3BufferAddValue(&ja3_elliptic_curves, elliptic_curve); if (rc != 0) return -1; @@ -867,7 +867,7 @@ static inline int TLSDecodeHSHelloExtensionEllipticCurvePF(SSLState *ssl_state, input += 1; if (TLSDecodeValueIsGREASE(elliptic_curve_pf) != 1) { - int rc = Ja3BufferAddValue(ja3_elliptic_curves_pf, + int rc = Ja3BufferAddValue(&ja3_elliptic_curves_pf, elliptic_curve_pf); if (rc != 0) return -1; @@ -998,7 +998,7 @@ static inline int TLSDecodeHSHelloExtensions(SSLState *ssl_state, if ((ssl_state->current_flags & SSL_AL_FLAG_STATE_CLIENT_HELLO) && ssl_config.enable_ja3) { if (TLSDecodeValueIsGREASE(ext_type) != 1) { - rc = Ja3BufferAddValue(ja3_extensions, ext_type); + rc = Ja3BufferAddValue(&ja3_extensions, ext_type); if (rc != 0) goto error; } @@ -1010,16 +1010,16 @@ static inline int TLSDecodeHSHelloExtensions(SSLState *ssl_state, end: if ((ssl_state->current_flags & SSL_AL_FLAG_STATE_CLIENT_HELLO) && ssl_config.enable_ja3) { - rc = Ja3BufferAppendBuffer(ssl_state->ja3_str, ja3_extensions); + rc = Ja3BufferAppendBuffer(&ssl_state->ja3_str, &ja3_extensions); if (rc == -1) goto error; - rc = Ja3BufferAppendBuffer(ssl_state->ja3_str, ja3_elliptic_curves); + rc = Ja3BufferAppendBuffer(&ssl_state->ja3_str, &ja3_elliptic_curves); if (rc == -1) goto error; - rc = Ja3BufferAppendBuffer(ssl_state->ja3_str, - ja3_elliptic_curves_pf); + rc = Ja3BufferAppendBuffer(&ssl_state->ja3_str, + &ja3_elliptic_curves_pf); if (rc == -1) goto error; } diff --git a/src/util-ja3.c b/src/util-ja3.c index 7898468fb0..eee87a02cf 100644 --- a/src/util-ja3.c +++ b/src/util-ja3.c @@ -106,39 +106,40 @@ static int Ja3BufferResizeIfFull(JA3Buffer *buffer, uint32_t len) * \retval 0 on success. * \retval -1 on failure. */ -int Ja3BufferAppendBuffer(JA3Buffer *buffer1, JA3Buffer *buffer2) +int Ja3BufferAppendBuffer(JA3Buffer **buffer1, JA3Buffer **buffer2) { - if (buffer1 == NULL || buffer2 == NULL) { + if (*buffer1 == NULL || *buffer2 == NULL) { SCLogError(SC_ERR_INVALID_ARGUMENT, "Buffers should not be NULL"); return -1; } /* If buffer1 contains no data, then we just copy the second buffer instead of appending its data. */ - if (buffer1->data == NULL) { - buffer1->data = buffer2->data; - buffer1->used = buffer2->used; - buffer1->size = buffer2->size; - SCFree(buffer2); + if ((*buffer1)->data == NULL) { + (*buffer1)->data = (*buffer2)->data; + (*buffer1)->used = (*buffer2)->used; + (*buffer1)->size = (*buffer2)->size; + SCFree(*buffer2); return 0; } - int rc = Ja3BufferResizeIfFull(buffer1, buffer2->used); + int rc = Ja3BufferResizeIfFull(*buffer1, (*buffer2)->used); if (rc != 0) { - Ja3BufferFree(&buffer1); - Ja3BufferFree(&buffer2); + Ja3BufferFree(buffer1); + Ja3BufferFree(buffer2); return -1; } - if (buffer2->used == 0) { - buffer1->used += snprintf(buffer1->data + buffer1->used, buffer1->size - - buffer1->used, ","); + if ((*buffer2)->used == 0) { + (*buffer1)->used += snprintf((*buffer1)->data + (*buffer1)->used, + (*buffer1)->size - (*buffer1)->used, ","); } else { - buffer1->used += snprintf(buffer1->data + buffer1->used, buffer1->size - - buffer1->used, ",%s", buffer2->data); + (*buffer1)->used += snprintf((*buffer1)->data + (*buffer1)->used, + (*buffer1)->size - (*buffer1)->used, ",%s", + (*buffer2)->data); } - Ja3BufferFree(&buffer2); + Ja3BufferFree(buffer2); return 0; } @@ -169,38 +170,40 @@ static uint32_t NumberOfDigits(uint32_t num) * \retval 0 on success. * \retval -1 on failure. */ -int Ja3BufferAddValue(JA3Buffer *buffer, uint32_t value) +int Ja3BufferAddValue(JA3Buffer **buffer, uint32_t value) { - if (buffer == NULL) { + if (*buffer == NULL) { SCLogError(SC_ERR_INVALID_ARGUMENT, "Buffer should not be NULL"); return -1; } - if (buffer->data == NULL) { - buffer->data = SCMalloc(JA3_BUFFER_INITIAL_SIZE * sizeof(char)); - if (buffer->data == NULL) { + if ((*buffer)->data == NULL) { + (*buffer)->data = SCMalloc(JA3_BUFFER_INITIAL_SIZE * sizeof(char)); + 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; + (*buffer)->size = JA3_BUFFER_INITIAL_SIZE; } uint32_t value_len = NumberOfDigits(value); - int rc = Ja3BufferResizeIfFull(buffer, value_len); + int rc = Ja3BufferResizeIfFull(*buffer, value_len); if (rc != 0) { - Ja3BufferFree(&buffer); + Ja3BufferFree(buffer); return -1; } - if (buffer->used == 0) { - buffer->used += snprintf(buffer->data, buffer->size, "%d", value); + if ((*buffer)->used == 0) { + (*buffer)->used += snprintf((*buffer)->data, (*buffer)->size, "%d", + value); } else { - buffer->used += snprintf(buffer->data + buffer->used, buffer->size - - buffer->used, "-%d", value); + (*buffer)->used += snprintf((*buffer)->data + (*buffer)->used, + (*buffer)->size - (*buffer)->used, "-%d", + value); } return 0; diff --git a/src/util-ja3.h b/src/util-ja3.h index e82d198884..ed3077d836 100644 --- a/src/util-ja3.h +++ b/src/util-ja3.h @@ -34,8 +34,8 @@ typedef struct JA3Buffer_ { JA3Buffer *Ja3BufferInit(void); void Ja3BufferFree(JA3Buffer **); -int Ja3BufferAppendBuffer(JA3Buffer *, JA3Buffer *); -int Ja3BufferAddValue(JA3Buffer *, uint32_t); +int Ja3BufferAppendBuffer(JA3Buffer **, JA3Buffer **); +int Ja3BufferAddValue(JA3Buffer **, uint32_t); char *Ja3GenerateHash(JA3Buffer *); int Ja3IsDisabled(const char *);