From 0f7052564c93a9600422af26e7f8e6e12da6b5d8 Mon Sep 17 00:00:00 2001 From: Panagiotis Vasilikos Date: Fri, 24 Jan 2020 16:34:56 +0100 Subject: [PATCH 1/3] Memory leak in handle_unsubscribe.c Reason: In line 70, the memory allocation for the pointer reasons_codes may result to a memory leak due to the many returns (e.g as the one in line 78) occuring in the program's path until reaching the mosquitto__free at line 122. Fix: I moved the memory allocation code block (lines 69-73) just before the line 102. This is the first place the pointer reason_codes is used, while the following mosquitto__free operators free the allocated memory correctly. Signed-off-by: Panagiotis Vasilikos --- src/handle_unsubscribe.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/handle_unsubscribe.c b/src/handle_unsubscribe.c index e9f89f8c2..16ff856d0 100644 --- a/src/handle_unsubscribe.c +++ b/src/handle_unsubscribe.c @@ -66,12 +66,6 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context) } } - reason_code_max = 10; - reason_codes = mosquitto__malloc(reason_code_max); - if(!reason_codes){ - return MOSQ_ERR_NOMEM; - } - while(context->in_packet.pos < context->in_packet.remaining_length){ sub = NULL; if(packet__read_string(&context->in_packet, &sub, &slen)){ @@ -99,6 +93,12 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context) mosquitto__free(sub); if(rc) return rc; + reason_code_max = 10; + reason_codes = mosquitto__malloc(reason_code_max); + if(!reason_codes){ + return MOSQ_ERR_NOMEM; + } + reason_codes[reason_code_count] = reason; reason_code_count++; if(reason_code_count == reason_code_max){ @@ -122,4 +122,3 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context) mosquitto__free(reason_codes); return rc; } - From caeb211cc5efdbb3755c4fd9bc68c83151c35de5 Mon Sep 17 00:00:00 2001 From: Panagiotis Vasilikos Date: Tue, 28 Jan 2020 11:34:11 +0100 Subject: [PATCH 2/3] Memory leak in socks_mosq.c Reason: The memory allocated for the packet pointer at line 155 is not freed before returning at line 188. Fix: I inserted the mosquitto__free(packet) statement just before returning at line 188. Signed-off-by: Panagiotis Vasilikos --- lib/socks_mosq.c | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/socks_mosq.c b/lib/socks_mosq.c index a9f0bbcd1..bb6515928 100644 --- a/lib/socks_mosq.c +++ b/lib/socks_mosq.c @@ -185,6 +185,7 @@ int socks5__send(struct mosquitto *mosq) }else{ slen = strlen(mosq->host); if(slen > UCHAR_MAX){ + mosquitto__free(packet); return MOSQ_ERR_NOMEM; } packet->packet_length = 7 + slen; From 49bf78886273d82241de91764ac183541d48ae06 Mon Sep 17 00:00:00 2001 From: Panagiotis Vasilikos Date: Tue, 28 Jan 2020 16:31:15 +0100 Subject: [PATCH 3/3] Memory leak in handle_unsubscribe.c Reason: In line 70, the memory allocation for the pointer reasons_codes may result to a memory leak due to the many returns (e.g as the one in line 78) occuring in the program's path until reaching the mosquitto__free at line 122. Fix: I added a mosquitto__free(reason_codes) statement before each return statement that could result to a memory leak Signed-off-by: Panagiotis Vasilikos --- src/handle_unsubscribe.c | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) diff --git a/src/handle_unsubscribe.c b/src/handle_unsubscribe.c index 16ff856d0..df2269838 100644 --- a/src/handle_unsubscribe.c +++ b/src/handle_unsubscribe.c @@ -66,9 +66,16 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context) } } + reason_code_max = 10; + reason_codes = mosquitto__malloc(reason_code_max); + if(!reason_codes){ + return MOSQ_ERR_NOMEM; + } + while(context->in_packet.pos < context->in_packet.remaining_length){ sub = NULL; if(packet__read_string(&context->in_packet, &sub, &slen)){ + mosquitto__free(reason_codes); return 1; } @@ -77,6 +84,7 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context) "Empty unsubscription string from %s, disconnecting.", context->id); mosquitto__free(sub); + mosquitto_free(reason_codes); return 1; } if(mosquitto_sub_topic_check(sub)){ @@ -84,6 +92,7 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context) "Invalid unsubscription string from %s, disconnecting.", context->id); mosquitto__free(sub); + mosquitto__free(reason_codes); return 1; } @@ -91,12 +100,9 @@ int handle__unsubscribe(struct mosquitto_db *db, struct mosquitto *context) rc = sub__remove(db, context, sub, db->subs, &reason); log__printf(NULL, MOSQ_LOG_UNSUBSCRIBE, "%s %s", context->id, sub); mosquitto__free(sub); - if(rc) return rc; - - reason_code_max = 10; - reason_codes = mosquitto__malloc(reason_code_max); - if(!reason_codes){ - return MOSQ_ERR_NOMEM; + if(rc){ + mosquitto_fee(reason_codes); + return rc; } reason_codes[reason_code_count] = reason;