Skip to content

Commit

Permalink
Fix segfault on client sending malformed CONNACk.
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
ralight committed Apr 3, 2021
1 parent 6ebbb4d commit 6a4a547
Show file tree
Hide file tree
Showing 5 changed files with 62 additions and 2 deletions.
10 changes: 9 additions & 1 deletion 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 #2163.

Clients:
- Set `receive-maximum` to not exceed the `-C` message count in mosquitto_sub
Expand Down
2 changes: 1 addition & 1 deletion src/handle_connack.c
Expand Up @@ -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);
Expand Down
50 changes: 50 additions & 0 deletions 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)
1 change: 1 addition & 0 deletions test/broker/Makefile
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions test/broker/test.py
Expand Up @@ -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'),
Expand Down

0 comments on commit 6a4a547

Please sign in to comment.