From fe0d92053365bdea24dbca455c0f15862a01361e Mon Sep 17 00:00:00 2001 From: "Roger A. Light" Date: Sat, 3 Apr 2021 11:07:30 +0100 Subject: [PATCH] Fix segfault on client sending malformed CONNACk. CVE-xxxx-xxxx: If an authenticated client connected with MQTT v5 sent a malformed CONNACK message to the broker a NULL pointer dereference occurred, most likely resulting in a segfault. This will be updated with the CVE number when it is assigned. Affects versions 2.0.0 to 2.0.9 inclusive. Closes #2163. Thanks to Bryan Pearson. --- ChangeLog.txt | 10 +++++- src/handle_connack.c | 2 +- test/broker/01-connect-connack-2163.py | 50 ++++++++++++++++++++++++++ test/broker/Makefile | 1 + test/broker/test.py | 1 + 5 files changed, 62 insertions(+), 2 deletions(-) create mode 100755 test/broker/01-connect-connack-2163.py diff --git a/ChangeLog.txt b/ChangeLog.txt index 27d3c73d3b..7da386bd79 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,9 +1,17 @@ -2.0.10 - 2021-xx-xx +2.0.10 - 2021-04-03 ================== +Security: +- CVE-xxxx-xxxx: If an authenticated client connected with MQTT v5 sent a + malformed CONNACK message to the broker a NULL pointer dereference occurred, + most likely resulting in a segfault. This will be updated with the CVE + number when it is assigned. + Affects versions 2.0.0 to 2.0.9 inclusive. + Broker: - Don't over write new receive-maximum if a v5 client connects and takes over an old session. Closes #2134. +- Fix CVE-xxxx-xxxx. Closes #2136. Clients: - Set `receive-maximum` to not exceed the `-C` message count in mosquitto_sub diff --git a/src/handle_connack.c b/src/handle_connack.c index 6aacb5926b..573a0ab0b9 100644 --- a/src/handle_connack.c +++ b/src/handle_connack.c @@ -39,7 +39,7 @@ int handle__connack(struct mosquitto *context) uint16_t server_keepalive; uint8_t max_qos = 255; - if(!context){ + if(context == NULL || context->bridge == NULL){ return MOSQ_ERR_INVAL; } log__printf(NULL, MOSQ_LOG_DEBUG, "Received CONNACK on connection %s.", context->id); diff --git a/test/broker/01-connect-connack-2163.py b/test/broker/01-connect-connack-2163.py new file mode 100755 index 0000000000..8f0297f1a4 --- /dev/null +++ b/test/broker/01-connect-connack-2163.py @@ -0,0 +1,50 @@ +#!/usr/bin/env python3 + +# Test https://github.com/eclipse/mosquitto/issues/2163 +# Does the broker cope with a malformed CONNACK sent to it after a valid CONNECT? + +from mosq_test_helper import * + +def do_test(proto_ver): + rc = 1 + keepalive = 10 + connect_packet = mosq_test.gen_connect("connect-connack-2163", keepalive=keepalive, proto_ver=proto_ver) + connack_packet = mosq_test.gen_connack(rc=0, proto_ver=proto_ver) + connack_malformed = struct.pack("BBBBB", 0x02, 0x00, 0x01, 0xE0, 0x00) + connack_malformed = struct.pack("BBBB", 0x29, 0x02, 0x00, 0x01) + pingreq_packet = mosq_test.gen_pingreq() + + 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, port=port) + sock.send(connack_malformed) + try: + mosq_test.do_send_receive(sock, pingreq_packet, b"", "pingreq") + except ConnectionResetError: + pass + sock.close() + + # Does the broker still exist? + sock = mosq_test.do_client_connect(connect_packet, connack_packet, port=port) + mosq_test.do_ping(sock) + sock.close() + + rc = 0 + except mosq_test.TestError: + pass + finally: + broker.terminate() + broker.wait() + (stdo, stde) = broker.communicate() + if rc: + print(stde.decode('utf-8')) + print("proto_ver=%d" % (proto_ver)) + exit(rc) + + +do_test(proto_ver=3) +do_test(proto_ver=4) +do_test(proto_ver=5) +exit(0) diff --git a/test/broker/Makefile b/test/broker/Makefile index 018160635d..c874d67b88 100644 --- a/test/broker/Makefile +++ b/test/broker/Makefile @@ -22,6 +22,7 @@ test : test-compile 01 02 03 04 05 06 07 08 09 10 11 12 13 14 01 : ./01-connect-allow-anonymous.py ./01-connect-bad-packet.py + ./01-connect-connack-2163.py ./01-connect-disconnect-v5.py ./01-connect-duplicate.py ./01-connect-invalid-id-0.py diff --git a/test/broker/test.py b/test/broker/test.py index 6221620316..abf94143e9 100755 --- a/test/broker/test.py +++ b/test/broker/test.py @@ -7,6 +7,7 @@ #(ports required, 'path'), (1, './01-connect-allow-anonymous.py'), (1, './01-connect-bad-packet.py'), + (1, './01-connect-connack-2163.py'), (1, './01-connect-disconnect-v5.py'), (1, './01-connect-duplicate.py'), (1, './01-connect-invalid-id-0.py'),