From fe5afb0779bac36f967865bfb1c72e900e365b57 Mon Sep 17 00:00:00 2001 From: EdouardMalot Date: Mon, 13 Jul 2026 17:07:08 +0200 Subject: [PATCH] Fix MQTT client: clear credentials and will on connection teardown nxd_mqtt_client_login_set stores pointers into the caller's buffers, and only _nxd_mqtt_process_disconnect reset them. Failed connects (CONNACK refused, TCP or TLS establish failure) tear down through _nxd_mqtt_client_connection_end and skip that reset, so a later connect without credentials sends CONNECT with the username flag set but no username. Brokers drop it as a protocol error. Move the cleanup into _nxd_mqtt_client_connection_end, which every teardown path goes through except the early TCP-failure branch of _nxd_mqtt_client_connect. That branch gets the same cleanup inline, plus a use_tls reset: it has the same stale-flag problem that #400 fixes in connection_end, on a path #400 does not cover. --- addons/mqtt/nxd_mqtt_client.c | 25 ++++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-) diff --git a/addons/mqtt/nxd_mqtt_client.c b/addons/mqtt/nxd_mqtt_client.c index 3f8e85677..d0989e162 100644 --- a/addons/mqtt/nxd_mqtt_client.c +++ b/addons/mqtt/nxd_mqtt_client.c @@ -1950,13 +1950,6 @@ UCHAR fixed_header; client_ptr -> nxd_mqtt_ping_sent_time = 0; } - /* Clean up the information when disconnecting. */ - client_ptr -> nxd_mqtt_client_username = NX_NULL; - client_ptr -> nxd_mqtt_client_password = NX_NULL; - client_ptr -> nxd_mqtt_client_will_topic = NX_NULL; - client_ptr -> nxd_mqtt_client_will_message = NX_NULL; - client_ptr -> nxd_mqtt_client_will_qos_retain = 0; - /* Release current processing packet. */ if (client_ptr -> nxd_mqtt_client_processing_packet) { @@ -2571,6 +2564,14 @@ VOID _nxd_mqtt_client_connection_end(NXD_MQTT_CLIENT *client_ptr, ULONG wait_opt nx_tcp_socket_disconnect(&(client_ptr -> nxd_mqtt_client_socket), wait_option); nx_tcp_client_socket_unbind(&(client_ptr -> nxd_mqtt_client_socket)); + /* Clean up per-connection information so a failed or ended connection + never leaks credentials or will settings into the next CONNECT. */ + client_ptr -> nxd_mqtt_client_username = NX_NULL; + client_ptr -> nxd_mqtt_client_password = NX_NULL; + client_ptr -> nxd_mqtt_client_will_topic = NX_NULL; + client_ptr -> nxd_mqtt_client_will_message = NX_NULL; + client_ptr -> nxd_mqtt_client_will_qos_retain = 0; + /* Disable timer if timer has been started. */ if (client_ptr -> nxd_mqtt_keepalive) { @@ -3750,9 +3751,19 @@ UINT old_priority; { nx_secure_tls_session_delete(&(client_ptr -> nxd_mqtt_tls_session)); } + client_ptr -> nxd_mqtt_client_use_tls = 0; #endif /* NX_SECURE_ENABLE */ nx_tcp_client_socket_unbind(&(client_ptr -> nxd_mqtt_client_socket)); tx_timer_delete(&(client_ptr -> nxd_mqtt_timer)); + + /* Same per-connection cleanup as _nxd_mqtt_client_connection_end — + this early-failure path is the only teardown that does not go + through it. */ + client_ptr -> nxd_mqtt_client_username = NX_NULL; + client_ptr -> nxd_mqtt_client_password = NX_NULL; + client_ptr -> nxd_mqtt_client_will_topic = NX_NULL; + client_ptr -> nxd_mqtt_client_will_message = NX_NULL; + client_ptr -> nxd_mqtt_client_will_qos_retain = 0; return(NXD_MQTT_CONNECT_FAILURE); }