Fix memory leak on handling QoS 2 PUBLISH.

In some circumstances, Mosquitto could leak memory when handling PUBLISH  messages. This is limited to incoming QoS 2 messages, and is related to the combination of the broker having persistence enabled, a clean session=false client, which was connected prior to the broker restarting, then has reconnected and has now sent messages at a sufficiently high rate that the incoming queue at the broker has filled up and hence messages are being dropped. This is more likely to have an effect where max_queued_messages is a small value. This has now been fixed.

Closes #1793. Thanks to mbates14.

ChangeLog.txt

@@ -1,3 +1,16 @@
+1.6.12 - 2020-08-19
+===================
+
+Security:
+- In some circumstances, Mosquitto could leak memory when handling PUBLISH
+ messages. This is limited to incoming QoS 2 messages, and is related
+ to the combination of the broker having persistence enabled, a clean
+ session=false client, which was connected prior to the broker restarting,
+ then has reconnected and has now sent messages at a sufficiently high rate
+ that the incoming queue at the broker has filled up and hence messages are
+ being dropped. This is more likely to have an effect where
+ max_queued_messages is a small value. This has now been fixed. Closes #1793.
+
 Broker:
 - Build warning fixes when building with WITH_BRIDGE=no and WITH_TLS=no.
 

src/database.c

@@ -37,7 +37,7 @@ static unsigned long max_queued_bytes = 0;
  * @param qos qos for the packet of interest
  * @return true if more in flight are allowed.
  */
-static bool db__ready_for_flight(struct mosquitto_msg_data *msgs, int qos)
+bool db__ready_for_flight(struct mosquitto_msg_data *msgs, int qos)
 {
  bool valid_bytes;
  bool valid_count;
@@ -68,7 +68,7 @@ static bool db__ready_for_flight(struct mosquitto_msg_data *msgs, int qos)
  * @param qos destination qos for the packet of interest
  * @return true if queuing is allowed, false if should be dropped
  */
-static bool db__ready_for_queue(struct mosquitto *context, int qos, struct mosquitto_msg_data *msg_data)
+bool db__ready_for_queue(struct mosquitto *context, int qos, struct mosquitto_msg_data *msg_data)
 {
  int source_count;
  int adjust_count;

src/handle_publish.c

@@ -302,12 +302,21 @@ int handle__publish(struct mosquitto_db *db, struct mosquitto *context)
  db__message_store_find(context, mid, &stored);
  }
  if(!stored){
- dup = 0;
- if(db__message_store(db, context, mid, topic, qos, payloadlen, &payload, retain, &stored, message_expiry_interval, msg_properties, 0, mosq_mo_client)){
- mosquitto_property_free_all(&msg_properties);
- return 1;
+ if(qos == 0
+ || db__ready_for_flight(&context->msgs_in, qos)
+ || db__ready_for_queue(context, qos, &context->msgs_in)){
+
+ dup = 0;
+ if(db__message_store(db, context, mid, topic, qos, payloadlen, &payload, retain, &stored, message_expiry_interval, msg_properties, 0, mosq_mo_client)){
+ mosquitto_property_free_all(&msg_properties);
+ return 1;
+ }
+ msg_properties = NULL; /* Now belongs to db__message_store() */
+ }else{
+ /* Client isn't allowed any more incoming messages, so fail early */
+ reason_code = MQTT_RC_QUOTA_EXCEEDED;
+ goto process_bad_message;
  }
- msg_properties = NULL; /* Now belongs to db__message_store() */
  }else{
  mosquitto__free(topic);
  topic = stored->topic;
@@ -340,6 +349,7 @@ int handle__publish(struct mosquitto_db *db, struct mosquitto *context)
  }
  /* db__message_insert() returns 2 to indicate dropped message
  * due to queue. This isn't an error so don't disconnect them. */
+ /* FIXME - this is no longer necessary due to failing early above */
  if(!res){
  if(send__pubrec(context, mid, 0)) rc = 1;
  }else if(res == 1){

src/mosquitto_broker_internal.h

@@ -629,6 +629,8 @@ void db__msg_store_ref_dec(struct mosquitto_db *db, struct mosquitto_msg_store *
 void db__msg_store_clean(struct mosquitto_db *db);
 void db__msg_store_compact(struct mosquitto_db *db);
 int db__message_reconnect_reset(struct mosquitto_db *db, struct mosquitto *context);
+bool db__ready_for_flight(struct mosquitto_msg_data *msgs, int qos);
+bool db__ready_for_queue(struct mosquitto *context, int qos, struct mosquitto_msg_data *msg_data);
 void sys_tree__init(struct mosquitto_db *db);
 void sys_tree__update(struct mosquitto_db *db, int interval, time_t start_time);