From a43cc056511433dd0ed5d4c797859223fd137f3f Mon Sep 17 00:00:00 2001 From: linsc Date: Tue, 25 Oct 2022 14:18:08 +0800 Subject: [PATCH 1/3] fix memory leak when gquic50 decrypt packet fix a suspicious memory leak in gquic and iquic --- src/liblsquic/lsquic_enc_sess_ietf.c | 10 +++++++--- src/liblsquic/lsquic_handshake.c | 11 +++++++---- 2 files changed, 14 insertions(+), 7 deletions(-) diff --git a/src/liblsquic/lsquic_enc_sess_ietf.c b/src/liblsquic/lsquic_enc_sess_ietf.c index ee6d3d4..772f045 100644 --- a/src/liblsquic/lsquic_enc_sess_ietf.c +++ b/src/liblsquic/lsquic_enc_sess_ietf.c @@ -2240,7 +2240,6 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p, enum enc_level enc_level; enum packnum_space pns; lsquic_packno_t packno; - size_t out_sz; enum dec_packin dec_packin; int s; /* 16Bytes: AEAD authentication tag @@ -2252,12 +2251,17 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p, * These cipher suites have a 16-byte authentication tag and * produce an output 16 bytes larger than their input. */ - const size_t dst_sz = packet_in->pi_data_sz - 16; + size_t out_sz, dst_sz; unsigned char new_secret[EVP_MAX_KEY_LENGTH]; struct crypto_ctx crypto_ctx_buf; char secret_str[EVP_MAX_KEY_LENGTH * 2 + 1]; char errbuf[ERR_ERROR_STRING_BUF_LEN]; + if (packet_in->pi_data_sz <= 16) { + dec_packin = DECPI_TOO_SHORT; + goto err; + } + dst_sz = packet_in->pi_data_sz - 16; dst = lsquic_mm_get_packet_in_buf(&enpub->enp_mm, dst_sz); if (!dst) { @@ -2450,10 +2454,10 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p, enc_sess->esi_key_phase = key_phase; } - packet_in->pi_data_sz = packet_in->pi_header_sz + out_sz; if (packet_in->pi_flags & PI_OWN_DATA) lsquic_mm_put_packet_in_buf(&enpub->enp_mm, packet_in->pi_data, packet_in->pi_data_sz); + packet_in->pi_data_sz = packet_in->pi_header_sz + out_sz; packet_in->pi_data = dst; packet_in->pi_flags |= PI_OWN_DATA | PI_DECRYPTED | (enc_level << PIBIT_ENC_LEV_SHIFT); diff --git a/src/liblsquic/lsquic_handshake.c b/src/liblsquic/lsquic_handshake.c index f6d3b35..2cfb2d9 100644 --- a/src/liblsquic/lsquic_handshake.c +++ b/src/liblsquic/lsquic_handshake.c @@ -4177,11 +4177,15 @@ gquic2_esf_decrypt_packet (enc_session_t *enc_session_p, unsigned sample_off, packno_len, divers_nonce_len; enum gel gel; lsquic_packno_t packno; - size_t out_sz; + size_t out_sz, dst_sz; enum dec_packin dec_packin; - const size_t dst_sz = packet_in->pi_data_sz; char errbuf[ERR_ERROR_STRING_BUF_LEN]; + dst_sz = packet_in->pi_data_sz - 16; + if (dst_sz <= 16) { + dec_packin = DECPI_TOO_SHORT; + goto err; + } dst = lsquic_mm_get_packet_in_buf(&enpub->enp_mm, dst_sz); if (!dst) { @@ -4270,11 +4274,10 @@ gquic2_esf_decrypt_packet (enc_session_t *enc_session_p, } /* Bits 2 and 3 are not set and don't need to be checked in gQUIC */ - - packet_in->pi_data_sz = packet_in->pi_header_sz + out_sz; if (packet_in->pi_flags & PI_OWN_DATA) lsquic_mm_put_packet_in_buf(&enpub->enp_mm, packet_in->pi_data, packet_in->pi_data_sz); + packet_in->pi_data_sz = packet_in->pi_header_sz + out_sz; packet_in->pi_data = dst; packet_in->pi_flags |= PI_OWN_DATA | PI_DECRYPTED | (gel2el[gel] << PIBIT_ENC_LEV_SHIFT); From 65e605a10d059807e2d0c0de4e7467daf72aa155 Mon Sep 17 00:00:00 2001 From: linsc Date: Tue, 15 Nov 2022 16:54:53 +0800 Subject: [PATCH 2/3] drop some redundant checking. --- src/liblsquic/lsquic_enc_sess_ietf.c | 8 ++------ src/liblsquic/lsquic_handshake.c | 8 ++------ 2 files changed, 4 insertions(+), 12 deletions(-) diff --git a/src/liblsquic/lsquic_enc_sess_ietf.c b/src/liblsquic/lsquic_enc_sess_ietf.c index 772f045..0288bb3 100644 --- a/src/liblsquic/lsquic_enc_sess_ietf.c +++ b/src/liblsquic/lsquic_enc_sess_ietf.c @@ -2240,6 +2240,7 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p, enum enc_level enc_level; enum packnum_space pns; lsquic_packno_t packno; + size_t out_sz; enum dec_packin dec_packin; int s; /* 16Bytes: AEAD authentication tag @@ -2251,17 +2252,12 @@ iquic_esf_decrypt_packet (enc_session_t *enc_session_p, * These cipher suites have a 16-byte authentication tag and * produce an output 16 bytes larger than their input. */ - size_t out_sz, dst_sz; + const size_t dst_sz = packet_in->pi_data_sz - IQUIC_TAG_LEN; unsigned char new_secret[EVP_MAX_KEY_LENGTH]; struct crypto_ctx crypto_ctx_buf; char secret_str[EVP_MAX_KEY_LENGTH * 2 + 1]; char errbuf[ERR_ERROR_STRING_BUF_LEN]; - if (packet_in->pi_data_sz <= 16) { - dec_packin = DECPI_TOO_SHORT; - goto err; - } - dst_sz = packet_in->pi_data_sz - 16; dst = lsquic_mm_get_packet_in_buf(&enpub->enp_mm, dst_sz); if (!dst) { diff --git a/src/liblsquic/lsquic_handshake.c b/src/liblsquic/lsquic_handshake.c index 2cfb2d9..964e48a 100644 --- a/src/liblsquic/lsquic_handshake.c +++ b/src/liblsquic/lsquic_handshake.c @@ -4177,15 +4177,11 @@ gquic2_esf_decrypt_packet (enc_session_t *enc_session_p, unsigned sample_off, packno_len, divers_nonce_len; enum gel gel; lsquic_packno_t packno; - size_t out_sz, dst_sz; + size_t out_sz; enum dec_packin dec_packin; + const size_t dst_sz = packet_in->pi_data_sz - IQUIC_TAG_LEN; char errbuf[ERR_ERROR_STRING_BUF_LEN]; - dst_sz = packet_in->pi_data_sz - 16; - if (dst_sz <= 16) { - dec_packin = DECPI_TOO_SHORT; - goto err; - } dst = lsquic_mm_get_packet_in_buf(&enpub->enp_mm, dst_sz); if (!dst) { From 8a823f20501144b71fe7ec620ed49343da6b7ae6 Mon Sep 17 00:00:00 2001 From: linsc Date: Thu, 24 Nov 2022 11:19:22 +0800 Subject: [PATCH 3/3] fix memory leak when gQUIC handshake --- src/liblsquic/lsquic_handshake.c | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/src/liblsquic/lsquic_handshake.c b/src/liblsquic/lsquic_handshake.c index 964e48a..5533441 100644 --- a/src/liblsquic/lsquic_handshake.c +++ b/src/liblsquic/lsquic_handshake.c @@ -1810,7 +1810,6 @@ get_valid_scfg (const struct lsquic_enc_session *enc_session, time_t t = time(NULL); unsigned int real_len; SCFG_info_t *temp_scfg; - SCFG_t *tmp_scfg_copy = NULL; void *scfg_ptr; int ret; unsigned msg_len, server_config_sz; @@ -1885,20 +1884,15 @@ get_valid_scfg (const struct lsquic_enc_session *enc_session, // /* TODO: will shi_delete call free to release the buffer? */ // shi->shi_delete(shi_ctx, SERVER_SCFG_KEY, SERVER_SCFG_KEY_SIZE); - void *scfg_key = strdup(SERVER_SCFG_KEY); - shi->shi_insert(shi_ctx, scfg_key, SERVER_SCFG_KEY_SIZE, + shi->shi_insert(shi_ctx, SERVER_SCFG_KEY, SERVER_SCFG_KEY_SIZE, enpub->enp_server_config->lsc_scfg, server_config_sz, t + settings->es_sttl); - ret = shi->shi_lookup(shi_ctx, scfg_key, SERVER_SCFG_KEY_SIZE, + ret = shi->shi_lookup(shi_ctx, SERVER_SCFG_KEY, SERVER_SCFG_KEY_SIZE, &scfg_ptr, &real_len); if (ret == 1) { - tmp_scfg_copy = scfg_ptr; - if (tmp_scfg_copy != enpub->enp_server_config->lsc_scfg) - { - free(enpub->enp_server_config->lsc_scfg); - enpub->enp_server_config->lsc_scfg = tmp_scfg_copy; - } + free(enpub->enp_server_config->lsc_scfg); + enpub->enp_server_config->lsc_scfg = scfg_ptr; } else {