diff --git a/CMakeLists.txt b/CMakeLists.txt index 7fc2595b..9840ec0a 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -11,7 +11,7 @@ project(mosquitto) cmake_minimum_required(VERSION 2.8) # Only for version 3 and up. cmake_policy(SET CMP0042 NEW) -set (VERSION 1.6.5) +set (VERSION 1.6.6) add_definitions (-DCMAKE -DVERSION=\"${VERSION}\") diff --git a/ChangeLog.txt b/ChangeLog.txt index 8fcb17d3..abf74289 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -1,3 +1,17 @@ +1.6.6 - 20190917 +================ + +Security: +- Restrict topic hierarchy to 200 levels to prevent possible stack overflow. + Closes #1412. + +Broker: +- Restrict topic hierarchy to 200 levels to prevent possible stack overflow. + Closes #1412. +- mosquitto_passwd now returns 1 when attempting to update a user that does + not exist. Closes #1414. + + 1.6.5 - 20190912 ================ diff --git a/config.mk b/config.mk index 471ed41c..a8a6debe 100644 --- a/config.mk +++ b/config.mk @@ -104,7 +104,7 @@ WITH_COVERAGE:=no # Also bump lib/mosquitto.h, CMakeLists.txt, # installer/mosquitto.nsi, installer/mosquitto64.nsi -VERSION=1.6.5 +VERSION=1.6.6 # Client library SO version. Bump if incompatible API/ABI changes are made. SOVERSION=1 diff --git a/installer/mosquitto.nsi b/installer/mosquitto.nsi index d59028b6..11d37731 100644 --- a/installer/mosquitto.nsi +++ b/installer/mosquitto.nsi @@ -9,7 +9,7 @@ !define env_hklm 'HKLM "SYSTEM\CurrentControlSet\Control\Session Manager\Environment"' Name "Eclipse Mosquitto" -!define VERSION 1.6.5 +!define VERSION 1.6.6 OutFile "mosquitto-${VERSION}-install-windows-x86.exe" InstallDir "$PROGRAMFILES\mosquitto" diff --git a/installer/mosquitto64.nsi b/installer/mosquitto64.nsi index c89d9a7d..c43a66a8 100644 --- a/installer/mosquitto64.nsi +++ b/installer/mosquitto64.nsi @@ -9,7 +9,7 @@ !define env_hklm 'HKLM "SYSTEM\CurrentControlSet\Control\Session Manager\Environment"' Name "Eclipse Mosquitto" -!define VERSION 1.6.5 +!define VERSION 1.6.6 OutFile "mosquitto-${VERSION}-install-windows-x64.exe" !include "x64.nsh" diff --git a/lib/mosquitto.h b/lib/mosquitto.h index c8116fa9..1e1dff2e 100644 --- a/lib/mosquitto.h +++ b/lib/mosquitto.h @@ -48,7 +48,7 @@ extern "C" { #define LIBMOSQUITTO_MAJOR 1 #define LIBMOSQUITTO_MINOR 6 -#define LIBMOSQUITTO_REVISION 5 +#define LIBMOSQUITTO_REVISION 6 /* LIBMOSQUITTO_VERSION_NUMBER looks like 1002001 for e.g. version 1.2.1. */ #define LIBMOSQUITTO_VERSION_NUMBER (LIBMOSQUITTO_MAJOR*1000000+LIBMOSQUITTO_MINOR*1000+LIBMOSQUITTO_REVISION) diff --git a/lib/util_topic.c b/lib/util_topic.c index 67b78782..673cc6ca 100644 --- a/lib/util_topic.c +++ b/lib/util_topic.c @@ -49,14 +49,25 @@ Contributors: int mosquitto_pub_topic_check(const char *str) { int len = 0; +#ifdef WITH_BROKER + int hier_count = 0; +#endif while(str && str[0]){ if(str[0] == '+' || str[0] == '#'){ return MOSQ_ERR_INVAL; } +#ifdef WITH_BROKER + else if(str[0] == '/'){ + hier_count++; + } +#endif len++; str = &str[1]; } if(len > 65535) return MOSQ_ERR_INVAL; +#ifdef WITH_BROKER + if(hier_count > TOPIC_HIERARCHY_LIMIT) return MOSQ_ERR_INVAL; +#endif return MOSQ_ERR_SUCCESS; } @@ -64,6 +75,9 @@ int mosquitto_pub_topic_check(const char *str) int mosquitto_pub_topic_check2(const char *str, size_t len) { size_t i; +#ifdef WITH_BROKER + int hier_count = 0; +#endif if(len > 65535) return MOSQ_ERR_INVAL; @@ -71,7 +85,15 @@ int mosquitto_pub_topic_check2(const char *str, size_t len) if(str[i] == '+' || str[i] == '#'){ return MOSQ_ERR_INVAL; } +#ifdef WITH_BROKER + else if(str[i] == '/'){ + hier_count++; + } +#endif } +#ifdef WITH_BROKER + if(hier_count > TOPIC_HIERARCHY_LIMIT) return MOSQ_ERR_INVAL; +#endif return MOSQ_ERR_SUCCESS; } @@ -87,6 +109,10 @@ int mosquitto_sub_topic_check(const char *str) { char c = '\0'; int len = 0; +#ifdef WITH_BROKER + int hier_count = 0; +#endif + while(str && str[0]){ if(str[0] == '+'){ if((c != '\0' && c != '/') || (str[1] != '\0' && str[1] != '/')){ @@ -97,11 +123,19 @@ int mosquitto_sub_topic_check(const char *str) return MOSQ_ERR_INVAL; } } +#ifdef WITH_BROKER + else if(str[0] == '/'){ + hier_count++; + } +#endif len++; c = str[0]; str = &str[1]; } if(len > 65535) return MOSQ_ERR_INVAL; +#ifdef WITH_BROKER + if(hier_count > TOPIC_HIERARCHY_LIMIT) return MOSQ_ERR_INVAL; +#endif return MOSQ_ERR_SUCCESS; } @@ -110,6 +144,9 @@ int mosquitto_sub_topic_check2(const char *str, size_t len) { char c = '\0'; size_t i; +#ifdef WITH_BROKER + int hier_count = 0; +#endif if(len > 65535) return MOSQ_ERR_INVAL; @@ -123,8 +160,16 @@ int mosquitto_sub_topic_check2(const char *str, size_t len) return MOSQ_ERR_INVAL; } } +#ifdef WITH_BROKER + else if(str[i] == '/'){ + hier_count++; + } +#endif c = str[i]; } +#ifdef WITH_BROKER + if(hier_count > TOPIC_HIERARCHY_LIMIT) return MOSQ_ERR_INVAL; +#endif return MOSQ_ERR_SUCCESS; } diff --git a/set-version.sh b/set-version.sh index 8c9f65e6..f7b82643 100755 --- a/set-version.sh +++ b/set-version.sh @@ -2,7 +2,7 @@ MAJOR=1 MINOR=6 -REVISION=5 +REVISION=6 sed -i "s/^VERSION=.*/VERSION=${MAJOR}.${MINOR}.${REVISION}/" config.mk diff --git a/snap/snapcraft.yaml b/snap/snapcraft.yaml index dec36750..91f7ed0d 100644 --- a/snap/snapcraft.yaml +++ b/snap/snapcraft.yaml @@ -1,5 +1,5 @@ name: mosquitto -version: 1.6.5 +version: 1.6.6 summary: Eclipse Mosquitto MQTT broker description: This is a message broker that supports version 3.1 and 3.1.1 of the MQTT protocol. diff --git a/src/mosquitto_broker_internal.h b/src/mosquitto_broker_internal.h index 25d4a5b0..322c6a8a 100644 --- a/src/mosquitto_broker_internal.h +++ b/src/mosquitto_broker_internal.h @@ -73,6 +73,9 @@ Contributors: #define WEBSOCKET_CLIENT -2 + +#define TOPIC_HIERARCHY_LIMIT 200 + /* ======================================== * UHPA data types * ======================================== */ diff --git a/src/mosquitto_passwd.c b/src/mosquitto_passwd.c index 00e048fb..41d32adb 100644 --- a/src/mosquitto_passwd.c +++ b/src/mosquitto_passwd.c @@ -223,6 +223,7 @@ int delete_pwuser(FILE *fptr, FILE *ftmp, const char *username) } if(!found){ fprintf(stderr, "Warning: User %s not found in password file.\n", username); + return 1; } return 0; } diff --git a/src/subs.c b/src/subs.c index aae32666..c059874f 100644 --- a/src/subs.c +++ b/src/subs.c @@ -220,6 +220,7 @@ static int sub__topic_tokenise(const char *subtopic, struct sub__token **topics) int start, stop, tlen; int i; char *topic; + int count = 0; assert(subtopic); assert(topics); @@ -242,6 +243,7 @@ static int sub__topic_tokenise(const char *subtopic, struct sub__token **topics) stop = 0; for(i=start; i TOPIC_HIERARCHY_LIMIT){ + /* Set limit on hierarchy levels, to restrict stack usage. */ + goto cleanup; + } + return MOSQ_ERR_SUCCESS; cleanup: diff --git a/test/broker/02-subscribe-long-topic.py b/test/broker/02-subscribe-long-topic.py new file mode 100755 index 00000000..c2e7a107 --- /dev/null +++ b/test/broker/02-subscribe-long-topic.py @@ -0,0 +1,36 @@ +#!/usr/bin/env python3 + +# Test whether a SUBSCRIBE to a topic with 65535 hierarchy characters fails +# This needs checking with MOSQ_USE_VALGRIND=1 to detect memory failures +# https://github.com/eclipse/mosquitto/issues/1412 + +from mosq_test_helper import * + +rc = 1 +mid = 1 +keepalive = 60 +connect_packet = mosq_test.gen_connect("subscribe-long-test", keepalive=keepalive) +connack_packet = mosq_test.gen_connack(rc=0) + +subscribe_packet = mosq_test.gen_subscribe(mid, "/"*65535, 0) +suback_packet = mosq_test.gen_suback(mid, 0) + +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) + mosq_test.do_send_receive(sock, subscribe_packet, b"", "suback") + + rc = 0 + + sock.close() +finally: + broker.terminate() + broker.wait() + (stdo, stde) = broker.communicate() + if rc: + print(stde.decode('utf-8')) + +exit(rc) + diff --git a/test/broker/03-publish-long-topic.py b/test/broker/03-publish-long-topic.py new file mode 100755 index 00000000..f57c33bb --- /dev/null +++ b/test/broker/03-publish-long-topic.py @@ -0,0 +1,37 @@ +#!/usr/bin/env python3 + +# Test whether a PUBLISH to a topic with 65535 hierarchy characters fails +# This needs checking with MOSQ_USE_VALGRIND=1 to detect memory failures +# https://github.com/eclipse/mosquitto/issues/1412 + + +from mosq_test_helper import * + +rc = 1 +mid = 19 +keepalive = 60 +connect_packet = mosq_test.gen_connect("pub-qos1-test", keepalive=keepalive) +connack_packet = mosq_test.gen_connack(rc=0) + +publish_packet = mosq_test.gen_publish("/"*65535, qos=1, mid=mid, payload="message") +puback_packet = mosq_test.gen_puback(mid) + +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) + mosq_test.do_send_receive(sock, publish_packet, b"", "puback") + + rc = 0 + + sock.close() +finally: + broker.terminate() + broker.wait() + (stdo, stde) = broker.communicate() + if rc: + print(stde.decode('utf-8')) + +exit(rc) + diff --git a/test/broker/07-will-delay-session-expiry2.py b/test/broker/07-will-delay-session-expiry2.py new file mode 100755 index 00000000..506dac01 --- /dev/null +++ b/test/broker/07-will-delay-session-expiry2.py @@ -0,0 +1,52 @@ +#!/usr/bin/env python3 + +# Test whether a client that connects with a will delay that is shorter than +# their session expiry interval has their will published. +# MQTT 5 +# https://github.com/eclipse/mosquitto/issues/1401 + +from mosq_test_helper import * + +rc = 1 +keepalive = 60 + +mid = 1 +connect1_packet = mosq_test.gen_connect("will-test", keepalive=keepalive, proto_ver=5) +connack1_packet = mosq_test.gen_connack(rc=0, proto_ver=5) + +will_props = mqtt5_props.gen_uint32_prop(mqtt5_props.PROP_WILL_DELAY_INTERVAL, 2) +connect_props = mqtt5_props.gen_uint32_prop(mqtt5_props.PROP_SESSION_EXPIRY_INTERVAL, 4) + +connect2_packet = mosq_test.gen_connect("will-helper", keepalive=keepalive, proto_ver=5, properties=connect_props, will_topic="will/test", will_payload=b"will delay", will_qos=2, will_properties=will_props) +connack2_packet = mosq_test.gen_connack(rc=0, proto_ver=5) + +subscribe_packet = mosq_test.gen_subscribe(mid, "will/test", 0, proto_ver=5) +suback_packet = mosq_test.gen_suback(mid, 0, proto_ver=5) + +publish_packet = mosq_test.gen_publish("will/test", qos=0, payload="will delay", proto_ver=5) + +port = mosq_test.get_port() +broker = mosq_test.start_broker(filename=os.path.basename(__file__), port=port) + +try: + sock1 = mosq_test.do_client_connect(connect1_packet, connack1_packet, timeout=30, port=port, connack_error="connack1") + mosq_test.do_send_receive(sock1, subscribe_packet, suback_packet, "suback") + + sock2 = mosq_test.do_client_connect(connect2_packet, connack2_packet, timeout=30, port=port, connack_error="connack2") + time.sleep(1) + sock2.close() + + # Wait for session to expire + time.sleep(3) + if mosq_test.expect_packet(sock1, "publish", publish_packet): + rc = 0 + + sock1.close() +finally: + broker.terminate() + broker.wait() + (stdo, stde) = broker.communicate() + if rc: + print(stde.decode('utf-8')) + exit(rc) + diff --git a/test/broker/11-message-expiry.py b/test/broker/11-message-expiry.py index b976d2fd..253b7ddb 100755 --- a/test/broker/11-message-expiry.py +++ b/test/broker/11-message-expiry.py @@ -75,7 +75,7 @@ try: sock.close() broker = mosq_test.start_broker(filename=os.path.basename(__file__), use_conf=True, port=port) - time.sleep(5) + time.sleep(7) sock = mosq_test.do_client_connect(connect_packet, connack2_packet, timeout=20, port=port) packet = sock.recv(len(publish2s_packet)) diff --git a/test/broker/Makefile b/test/broker/Makefile index fb6cda1e..e4c32c63 100644 --- a/test/broker/Makefile +++ b/test/broker/Makefile @@ -75,6 +75,7 @@ endif ./02-subpub-qos2.py ./02-subscribe-dollar-v5.py ./02-subscribe-invalid-utf8.py + ./02-subscribe-long-topic.py ./02-subscribe-persistence-flipflop.py ./02-subscribe-qos0.py ./02-subscribe-qos1.py @@ -99,6 +100,7 @@ endif ./03-publish-dollar-v5.py ./03-publish-dollar.py ./03-publish-invalid-utf8.py + ./03-publish-long-topic.py ./03-publish-qos1-no-subscribers-v5.py ./03-publish-qos1-retain-disabled.py ./03-publish-qos1.py @@ -139,6 +141,7 @@ endif ./07-will-delay-reconnect.py ./07-will-delay-recover.py ./07-will-delay-session-expiry.py + ./07-will-delay-session-expiry2.py ./07-will-delay.py ./07-will-disconnect-with-will.py ./07-will-invalid-utf8.py diff --git a/test/broker/test.py b/test/broker/test.py index 5aadbbee..7a2f9f66 100755 --- a/test/broker/test.py +++ b/test/broker/test.py @@ -54,6 +54,7 @@ tests = [ (1, './02-subpub-qos2.py'), (1, './02-subscribe-dollar-v5.py'), (1, './02-subscribe-invalid-utf8.py'), + (1, './02-subscribe-long-topic.py'), (1, './02-subscribe-persistence-flipflop.py'), (1, './02-subscribe-qos0.py'), (1, './02-subscribe-qos1.py'), @@ -77,6 +78,7 @@ tests = [ (1, './03-publish-dollar-v5.py'), (1, './03-publish-dollar.py'), (1, './03-publish-invalid-utf8.py'), + (1, './03-publish-long-topic.py'), (1, './03-publish-qos1-no-subscribers-v5.py'), (1, './03-publish-qos1-retain-disabled.py'), (1, './03-publish-qos1.py'), @@ -113,6 +115,7 @@ tests = [ (1, './07-will-delay-reconnect.py'), (1, './07-will-delay-recover.py'), (1, './07-will-delay-session-expiry.py'), + (1, './07-will-delay-session-expiry2.py'), (1, './07-will-delay.py'), (1, './07-will-disconnect-with-will.py'), (1, './07-will-invalid-utf8.py'), diff --git a/test/mosq_test.py b/test/mosq_test.py index 2093e326..1669f566 100644 --- a/test/mosq_test.py +++ b/test/mosq_test.py @@ -478,19 +478,25 @@ def gen_pubrel(mid, dup=False, proto_ver=4, reason_code=-1, properties=None): def gen_pubcomp(mid, proto_ver=4, reason_code=-1, properties=None): return _gen_command_with_mid(112, mid, proto_ver, reason_code, properties) + def gen_subscribe(mid, topic, qos, proto_ver=4, properties=b""): topic = topic.encode("utf-8") + packet = struct.pack("!B", 130) if proto_ver == 5: if properties == b"": - pack_format = "!BBHBH"+str(len(topic))+"sB" - return struct.pack(pack_format, 130, 2+1+2+len(topic)+1, mid, 0, len(topic), topic, qos) + packet += pack_remaining_length(2+1+2+len(topic)+1) + pack_format = "!HBH"+str(len(topic))+"sB" + return packet + struct.pack(pack_format, mid, 0, len(topic), topic, qos) else: properties = mqtt5_props.prop_finalise(properties) - pack_format = "!BBH"+str(len(properties))+"s"+"H"+str(len(topic))+"sB" - return struct.pack(pack_format, 130, 2+1+2+len(topic)+len(properties), mid, properties, len(topic), topic, qos) + packet += pack_remaining_length(2+1+2+len(topic)+len(properties)) + pack_format = "!H"+str(len(properties))+"s"+"H"+str(len(topic))+"sB" + return packet + struct.pack(pack_format, mid, properties, len(topic), topic, qos) else: - pack_format = "!BBHH"+str(len(topic))+"sB" - return struct.pack(pack_format, 130, 2+2+len(topic)+1, mid, len(topic), topic, qos) + packet += pack_remaining_length(2+2+len(topic)+1) + pack_format = "!HH"+str(len(topic))+"sB" + return packet + struct.pack(pack_format, mid, len(topic), topic, qos) + def gen_suback(mid, qos, proto_ver=4): if proto_ver == 5: diff --git a/www/pages/download.md b/www/pages/download.md index 5499441d..a18abc72 100644 --- a/www/pages/download.md +++ b/www/pages/download.md @@ -1,7 +1,7 @@ + +Mosquitto 1.6.6 and 1.5.9 have been released to address two security vulnerabilities. + +Titles and links will be updated once the CVE numbers are assigned. + +# CVE-xxxx-xxxxx + +A vulnerability exists in Mosquitto versions 1.5 to 1.6.5 inclusive. + +If a client sends a SUBSCRIBE packet containing a topic that consists of +approximately 65400 or more '/' characters, i.e. the topic hierarchy separator, +then a stack overflow will occur. + +The issue is fixed in Mosquitto 1.6.6 and 1.5.9. Patches for older versions are +available at + +The fix addresses the problem by restricting the allowed number of topic +hierarchy levels to 200. An alternative fix is to increase the size of the +stack by a small amount. + +# CVE-yyyy-yyyyy + +A vulnerability exists in Mosquitto version 1.6 to 1.6.4 inclusive. + +If an MQTT v5 client connects to Mosquitto, sets a last will and testament, +sets a will delay interval, sets a session expiry interval, and the will delay +interval is set longer than the session expiry interval, then a use after free +error occurs, which has the potential to cause a crash in some situations. + +The issue is fixed in Mosquitto 1.6.5. Patches for older versions are available +at + +# Version 1.6.6 Changes + +The complete list of fixes addressed in version 1.6.6 is: + +## Security + +* Restrict topic hierarchy to 200 levels to prevent possible stack overflow. + Closes [#1412]. + +## Broker +* Restrict topic hierarchy to 200 levels to prevent possible stack overflow. + Closes [#1412]. +* `mosquitto_passwd` now returns 1 when attempting to update a user that does + not exist. Closes [#1414]. + +[#1412]: https://github.com/eclipse/mosquitto/issues/1412 +[#1414]: https://github.com/eclipse/mosquitto/issues/1414