Fix memory leak that could be caused by a malicious CONNECT packet.

Closes #533493 (on Eclipse bugtracker)

Thanks to Daniel Romero.

ChangeLog.txt

@@ -1,6 +1,10 @@
 1.5 - 2018xxxx
 ==============
 
+Security:
+- Fix memory leak that could be caused by a malicious CONNECT packet. This
+ does not yet have a CVE assigned. Closes #533493 (on Eclipse bugtracker)
+
 Broker features:
 - Add per_listener_settings to allow authentication and access control to be
  per listener.

src/handle_connect.c

@@ -108,7 +108,7 @@ void connection_check_acl(struct mosquitto_db *db, struct mosquitto *context, st
 
 int 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;
@@ -124,6 +124,7 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
  struct mosquitto__acl_user *acl_tail;
  struct mosquitto *found_context;
  int slen;
+ uint16_t slen16;
  struct mosquitto__subleaf *leaf;
  int i;
  struct mosquitto__security_options *security_opts;
@@ -145,20 +146,27 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
  goto handle_connect_error;
  }
 
- if(packet__read_string(&context->in_packet, &protocol_name, &slen)){
+ /* 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(packet__read_uint16(&context->in_packet, &slen16)){
  rc = 1;
  goto handle_connect_error;
- return 1;
  }
- if(!protocol_name){
- rc = 3;
+ slen = slen16;
+ if(slen != 4 /* MQTT */ && slen != 6 /* MQIsdp */){
+ rc = MOSQ_ERR_PROTOCOL;
  goto handle_connect_error;
- return 3;
  }
+ if(packet__read_bytes(&context->in_packet, protocol_name, slen)){
+ rc = MOSQ_ERR_PROTOCOL;
+ goto handle_connect_error;
+ }
+ protocol_name[slen] = '\0';
+
  if(packet__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){
@@ -195,8 +203,6 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
  rc = MOSQ_ERR_PROTOCOL;
  goto handle_connect_error;
  }
- mosquitto__free(protocol_name);
- protocol_name = NULL;
 
  if(packet__read_byte(&context->in_packet, &connect_flags)){
  rc = 1;
@@ -672,7 +678,6 @@ int handle__connect(struct mosquitto_db *db, struct mosquitto *context)
  mosquitto__free(will_payload);
  mosquitto__free(will_topic);
  mosquitto__free(will_struct);
- mosquitto__free(protocol_name);
 #ifdef WITH_TLS
  if(client_cert) X509_free(client_cert);
 #endif