From 1d18b3e3c76b0f7a17d1d2aeaf2d994b84f85ae8 Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Tue, 8 Jun 2021 16:46:22 +0100 Subject: [PATCH] Fix leak on crafted MQTT v5 CONNECT. If a MQTT v5 client connects with a crafted CONNECT packet a memory leak will occur. Thanks to Kathrin Kleinhammer. --- ChangeLog.txt | 8 ++++++ src/handle_connect.c | 2 ++ test/broker/07-will-delay-invalid-573191.py | 32 +++++++++++++++++++++ test/broker/Makefile | 1 + test/broker/test.py | 1 + 5 files changed, 44 insertions(+) create mode 100755 test/broker/07-will-delay-invalid-573191.py diff --git a/ChangeLog.txt b/ChangeLog.txt index 12692ef9..3b1ef0d7 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,3 +1,11 @@ +1.6.15 - 2021-06-08 +=================== + +Security: +- If a MQTT v5 client connects with a crafted CONNECT packet a memory leak + will occur. This has been fixed. + + 1.6.14 - 2021-02-04 =================== diff --git a/src/handle_connect.c b/src/handle_connect.c index 6a9e198a..c0bb5d14 100644 --- a/src/handle_connect.c +++ b/src/handle_connect.c @@ -882,11 +882,13 @@ handle_connect_error: mosquitto__free(will_struct->msg.topic); mosquitto__free(will_struct); } + context->will = NULL; #ifdef WITH_TLS if(client_cert) X509_free(client_cert); #endif /* We return an error here which means the client is freed later on. */ context->clean_start = true; context->session_expiry_interval = 0; + context->will_delay_interval = 0; return rc; } diff --git a/test/broker/07-will-delay-invalid-573191.py b/test/broker/07-will-delay-invalid-573191.py new file mode 100755 index 00000000..18f4acfc --- /dev/null +++ b/test/broker/07-will-delay-invalid-573191.py @@ -0,0 +1,32 @@ +#!/usr/bin/env python3 + +# Test for https://bugs.eclipse.org/bugs/show_bug.cgi?id=573191 +# Check under valgrind/asan for leaks. + +from mosq_test_helper import * + +def do_test(): + rc = 1 + keepalive = 60 + + mid = 1 + props = mqtt5_props.gen_uint32_prop(mqtt5_props.PROP_WILL_DELAY_INTERVAL, 3) + connect_packet = mosq_test.gen_connect("will-573191-test", keepalive=keepalive, proto_ver=5, will_topic="", will_properties=props) + connack_packet = b"" + + port = mosq_test.get_port() + broker = mosq_test.start_broker(filename=os.path.basename(__file__), port=port) + + try: + sock = mosq_test.do_client_connect(connect_packet, connack_packet, timeout=30, port=port) + sock.close() + rc = 0 + finally: + broker.terminate() + broker.wait() + (stdo, stde) = broker.communicate() + if rc: + print(stde.decode('utf-8')) + exit(rc) + +do_test() diff --git a/test/broker/Makefile b/test/broker/Makefile index e26a06b3..fa889ce4 100644 --- a/test/broker/Makefile +++ b/test/broker/Makefile @@ -140,6 +140,7 @@ endif ./06-bridge-reconnect-local-out.py 07 : + ./07-will-delay-invalid-573191.py ./07-will-delay-reconnect.py ./07-will-delay-recover.py ./07-will-delay-session-expiry.py diff --git a/test/broker/test.py b/test/broker/test.py index b914f4cc..9a222624 100755 --- a/test/broker/test.py +++ b/test/broker/test.py @@ -114,6 +114,7 @@ tests = [ (3, './06-bridge-per-listener-settings.py'), (2, './06-bridge-reconnect-local-out.py'), + (1, './07-will-delay-invalid-573191.py'), (1, './07-will-delay-reconnect.py'), (1, './07-will-delay-recover.py'), (1, './07-will-delay-session-expiry.py'),