diff --git a/lib/util_mosq.c b/lib/util_mosq.c index 25bd61da..cfc3ce81 100644 --- a/lib/util_mosq.c +++ b/lib/util_mosq.c @@ -143,14 +143,25 @@ uint16_t mosquitto__mid_generate(struct mosquitto *mosq) 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; } @@ -158,6 +169,9 @@ int mosquitto_pub_topic_check(const char *str) int mosquitto_pub_topic_check2(const char *str, size_t len) { int i; +#ifdef WITH_BROKER + int hier_count = 0; +#endif if(len > 65535) return MOSQ_ERR_INVAL; @@ -165,7 +179,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; } @@ -181,6 +203,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] != '/')){ @@ -191,11 +217,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; } @@ -204,6 +238,9 @@ int mosquitto_sub_topic_check2(const char *str, size_t len) { char c = '\0'; int i; +#ifdef WITH_BROKER + int hier_count = 0; +#endif if(len > 65535) return MOSQ_ERR_INVAL; @@ -217,8 +254,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/src/mosquitto_broker_internal.h b/src/mosquitto_broker_internal.h index 512937ab..5c077cb4 100644 --- a/src/mosquitto_broker_internal.h +++ b/src/mosquitto_broker_internal.h @@ -70,6 +70,9 @@ Contributors: #define WEBSOCKET_CLIENT -2 + +#define TOPIC_HIERARCHY_LIMIT 200 + /* ======================================== * UHPA data types * ======================================== */ diff --git a/src/subs.c b/src/subs.c index 6b53aa60..a03c7bd0 100644 --- a/src/subs.c +++ b/src/subs.c @@ -178,6 +178,7 @@ static int sub__topic_tokenise(const char *subtopic, struct sub__token **topics) int start, stop, tlen; int i; mosquitto__topic_element_uhpa topic; + int count = 0; assert(subtopic); assert(topics); @@ -200,6 +201,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..d5a5f87c --- /dev/null +++ b/test/broker/02-subscribe-long-topic.py @@ -0,0 +1,44 @@ +#!/usr/bin/env python + +# 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 + +import time +import inspect, os, sys +# From http://stackoverflow.com/questions/279237/python-import-a-module-from-a-folder +cmd_subfolder = os.path.realpath(os.path.abspath(os.path.join(os.path.split(inspect.getfile( inspect.currentframe() ))[0],".."))) +if cmd_subfolder not in sys.path: + sys.path.insert(0, cmd_subfolder) + +import mosq_test + + +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..a7a06822 --- /dev/null +++ b/test/broker/03-publish-long-topic.py @@ -0,0 +1,45 @@ +#!/usr/bin/env python + +# 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 + + +import time +import inspect, os, sys +# From http://stackoverflow.com/questions/279237/python-import-a-module-from-a-folder +cmd_subfolder = os.path.realpath(os.path.abspath(os.path.join(os.path.split(inspect.getfile( inspect.currentframe() ))[0],".."))) +if cmd_subfolder not in sys.path: + sys.path.insert(0, cmd_subfolder) + +import mosq_test + + +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/Makefile b/test/broker/Makefile index 37601103..f1e17388 100644 --- a/test/broker/Makefile +++ b/test/broker/Makefile @@ -39,6 +39,7 @@ endif 02 : + ./02-subscribe-long-topic.py ./02-subscribe-qos0.py ./02-subscribe-qos1.py ./02-subscribe-qos2.py @@ -67,6 +68,7 @@ endif ./03-pattern-matching.py #./03-publish-qos1-queued-bytes.py ./03-publish-invalid-utf8.py + ./03-publish-long-topic.py ./03-publish-dollar.py 04 : diff --git a/test/broker/ptest.py b/test/broker/ptest.py index 74c441d9..33737bb6 100755 --- a/test/broker/ptest.py +++ b/test/broker/ptest.py @@ -25,6 +25,7 @@ tests = [ (1, './02-subscribe-qos0.py'), (1, './02-subscribe-qos1.py'), (1, './02-subscribe-qos2.py'), + (1, './02-subscribe-long-topic.py'), (1, './02-subpub-qos0.py'), (1, './02-subpub-qos1.py'), (1, './02-subpub-qos2.py'), @@ -46,6 +47,7 @@ tests = [ (1, './03-publish-b2c-disconnect-qos1.py'), (1, './03-publish-c2b-disconnect-qos2.py'), (1, './03-publish-b2c-disconnect-qos2.py'), + (1, './03-publish-long-topic.py'), (1, './03-pattern-matching.py'), #(1, './03-publish-qos1-queued-bytes.py'), (1, './03-publish-invalid-utf8.py'), diff --git a/test/mosq_test.py b/test/mosq_test.py index 10323323..180da04a 100644 --- a/test/mosq_test.py +++ b/test/mosq_test.py @@ -395,9 +395,13 @@ def gen_pubrel(mid, dup=False): def gen_pubcomp(mid): return struct.pack('!BBH', 112, 2, mid) + def gen_subscribe(mid, topic, qos): - pack_format = "!BBHH"+str(len(topic))+"sB" - return struct.pack(pack_format, 130, 2+2+len(topic)+1, mid, len(topic), topic, qos) + packet = struct.pack("!B", 130) + packet += bytes(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): return struct.pack('!BBHB', 144, 2+1, mid, qos) @@ -419,7 +423,7 @@ def gen_disconnect(): return struct.pack('!BB', 224, 0) def pack_remaining_length(remaining_length): - s = "" + s = bytes("".encode('utf-8')) while True: byte = remaining_length % 128 remaining_length = remaining_length // 128