diff --git a/ChangeLog.txt b/ChangeLog.txt index 33646461..c7dcb03f 100644 --- a/ChangeLog.txt +++ b/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. diff --git a/src/database.c b/src/database.c index 124824cd..1e58f05d 100644 --- a/src/database.c +++ b/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; diff --git a/src/handle_publish.c b/src/handle_publish.c index acbbec93..610a7ad8 100644 --- a/src/handle_publish.c +++ b/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){ diff --git a/src/mosquitto_broker_internal.h b/src/mosquitto_broker_internal.h index 1e0c42c6..ba42bafa 100644 --- a/src/mosquitto_broker_internal.h +++ b/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);