Vendor: Eclipse Mosquitto
Vendor URL: https://mosquitto.org/
Versions affected: <= 1.4.15
Systems Affected: Mosquitto Broker
Author: Daniel Romero – daniel.romero[at]nccgroup[dot]trust
Advisory URL / CVE Identifier: CVE-2017-7654
Risk: High (The memory leak vulnerability can lead to a Denial of Service)
Summary
A Memory Leak vulnerability was found within the Mosquitto Broker. Unauthenticated clients can send crafted CONNECT packets which could cause a denial of service in the Mosquitto Broker.
Location
A memory leak vulnerability was found within the Mosquitto Broker (src/read_handle_server.c) file, which when using crafted CONNECT messages a malicious (and unauthenticated) user could carry out denial of service attacks.
The affected code is shown below:
File: src/read_handle_server.c
77 int mqtt3_handle_connect(struct mosquitto_db *db, struct mosquitto *context)
78 {
79 char *protocol_name = NULL;
80 uint8_t protocol_version;
107
108 /* Don't accept multiple CONNECT commands. */
109 if(context->state != mosq_cs_new){
110 rc = MOSQ_ERR_PROTOCOL;
111 goto handle_connect_error;
112 }
113
114 if(_mosquitto_read_string( context->in_packet, protocol_name)){
115 rc = 1;
116 goto handle_connect_error;
117 return 1;
118 }
119 if(!protocol_name){
120 rc = 3;
121 goto handle_connect_error;
122 return 3;
123 }
124 if(_mosquitto_read_byte( context->in_packet, protocol_version)){
125 rc = 1;
126 goto handle_connect_error;
127 return 1;
128 }
1) Line 77: The mqtt3_handle_connect() Mosquitto broker function parsed a CONNECT packet. (from client to broker)
2) Line 114: The “protocol_name” (memory leak) was read and stored into its pointer.
3) Line 124: The “protocol_version” pointer attempted to be stored but it failed, so the IF statement was executed.
As the IF statement (line 124) did not implement the free() of the “protocol_name” pointer, it led in a memory leak vulnerability.
Impact
The memory leak vulnerability can lead to a Denial of Service.
Details
This memory leak vulnerability could be triggered by sending the following CONNECT packet structure: ID + PACKET_LENGTH + LEN_PROTO_NAME + PROTO_NAME. The MQTT protocol name allows storing up to 0xFFFF (65.535) bytes on each CONNECT packet.
An example of this vulnerability can be seen below (this memory leak vulnerability was checked against both the Mosquitto-1.4.15 and the last GIT version):
# valgrind --leak-check=full ./mosquitto -c mosquitto.conf > /dev/null
==19099== HEAP SUMMARY:
==19099== in use at exit: 224,103,740 bytes in 3,736 blocks
==19099== total heap usage: 31,957 allocs, 28,221 frees, 454,208,466 bytes allocated
==19099==
==19099== 5 bytes in 1 blocks are definitely lost in loss record 1 of 3
==19099== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==19099== by 0x5ECC389: strdup (strdup.c:42)
==19099== by 0x40D4A5: _mosquitto_strdup (memory_mosq.c:113)
==19099== by 0x40A22B: _conf_parse_string (conf.c:1821)
==19099== by 0x4081CE: _config_read_file_core (conf.c:0)
==19099== by 0x406BE0: _config_read_file (conf.c:1771)
==19099== by 0x406BE0: mqtt3_config_read (conf.c:474)
==19099== by 0x40683A: mqtt3_config_parse_args (conf.c:338)
==19099== by 0x40475E: main (mosquitto.c:263)
==19099==
==19099== 4,140,069 bytes in 69 blocks are possibly lost in loss record 2 of 3 <<< MEM LEAK
==19099== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==19099== by 0x40D3E5: _mosquitto_malloc (memory_mosq.c:67)
==19099== by 0x4117D7: _mosquitto_read_string (net_mosq.c:712)
==19099== by 0x412995: mqtt3_handle_connect (read_handle_server.c:115)
==19099== by 0x411C7C: _mosquitto_packet_read (net_mosq.c:1135)
==19099== by 0x40CF7A: loop_handle_reads_writes (loop.c:522)
==19099== by 0x40CF7A: mosquitto_main_loop (loop.c:359)
==19099== by 0x404B4B: main (mosquitto.c:385)
==19099==
==19099== 219,963,666 bytes in 3,666 blocks are definitely lost in loss record 3 of 3 <<< MEM LEAK
==19099== at 0x4C2BBAF: malloc (vg_replace_malloc.c:299)
==19099== by 0x40D3E5: _mosquitto_malloc (memory_mosq.c:67)
==19099== by 0x4117D7: _mosquitto_read_string (net_mosq.c:712)
==19099== by 0x412995: mqtt3_handle_connect (read_handle_server.c:115)
==19099== by 0x411C7C: _mosquitto_packet_read (net_mosq.c:1135)
==19099== by 0x40CF7A: loop_handle_reads_writes (loop.c:522)
==19099== by 0x40CF7A: mosquitto_main_loop (loop.c:359)
==19099== by 0x404B4B: main (mosquitto.c:385)
==19099==
==19099== LEAK SUMMARY:
==19099== definitely lost: 219,963,671 bytes in 3,667 blocks
==19099== indirectly lost: 0 bytes in 0 blocks
==19099== possibly lost: 4,140,069 bytes in 69 blocks
==19099== still reachable: 0 bytes in 0 blocks
==19099== suppressed: 0 bytes in 0 blocks
==19099==
==19099== For counts of detected and suppressed errors, rerun with: -v
==19099== ERROR SUMMARY: 3 errors from 3 contexts (suppressed: 0 from 0)
Recommendation
The following patch was applied:
<
pre>diff –git a/src/read_handle_server.c b/src/read_handle_server.c
index a16f205..57de06b 100644
— a/src/read_handle_server.c
+++ b/src/read_handle_server.c
@@ -76,7 +76,7 @@ static char client_id_gen(struct mosquitto_db *db)
int mqtt3_handle_connect(struct mosquitto_db *db, struct mosquitto *context)
{
– char *protocol_name = NULL;
+ char protocol_name[7];
uint8_t protocol_version;
uint8_t connect_flags;
uint8_t connect ack = 0;
@@ -111,20 +111,27 @@ int mqtt3_handle_connect(struct mosquitto_db *db, struct mosquitto
context)
goto handle_connect_error;
}
– if(_mosquitto_read_string( context->in_packet, protocol_name)){
+ /* Read protocol name as length then bytes rather than with read_string
+ * because the length is fixed and we can check that. Removes the need for
+ * another malloc as well. */
+ if(_mosquitto_read_uint16(packet, slen)){
rc = 1;
goto handle_connect_error;
– return 1;
}
– if(!protocol_name){
– rc = 3;
+ if(slen != 4 /* MQTT */ slen != 6 /* MQIsdp */){
+ rc = MOSQ_ERR_PROTOCOL;
+ goto handle_connect_error;
+ }
+ protocol_name[slen-1] = ‘ ‘;
+
+ if(_mosquitto_read_bytes( context->in_packet, protocol_name, slen)){
+ rc = MOSQ_ERR_PROTOCOL;
goto handle_connect_error;
– return 3;
}
+
if(_mosquitto_read_byte( context->in_packet, protocol_version)){
rc = 1;
goto handle_connect_error;
– return 1;
}
if(!strcmp(protocol_name, PROTOCOL_NAME_v31)){
if((protocol_version 0x7F) != PROTOCOL_VERSION_v31){
@@ -133,7 +140,6 @@ int mqtt3_handle_connect(struct mosquitto_db *db, struct mosquitto *context)
protocol_version, context->address);
}
_mosquitto_send_connack(context, 0, CONNACK_REFUSED_PROTOCOL_VERSION);
– _mosquitto_free(protocol_name);
rc = MOSQ_ERR_PROTOCOL;
goto handle_connect_error;
}
@@ -145,13 +151,11 @@ int mqtt3_handle_connect(struct mosquitto_db *db, struct mosquitto *context)
protocol_version, context->address);
}
_mosquitto_send_connack(context, 0, CONNACK_REFUSED_PROTOCOL_VERSION);
– _mosquitto_free(protocol_name);
rc = MOSQ_ERR_PROTOCOL;
goto handle_connect_error;
}
if((context->in_packet.command 0x0F) != 0x00){
/* Reserved flags not set to 0, must disconnect. */
– _mosquitto_free(protocol_name);
rc = MOSQ_ERR_PROTOCOL;
goto handle_connect_error;
}
@@ -161,11 +165,9 @@ int mqtt3_handle_connect(struct mosquitto_db *db, struct mosquitto *context)
_mosquitto_log_printf(NULL, MOSQ_LOG_INFO, “Invalid protocol “%s”” in CONNECT from %s.””
