diff --git a/ChangeLog.txt b/ChangeLog.txt index c7146ff549..215ca2041f 100644 --- a/ChangeLog.txt +++ b/ChangeLog.txt @@ -11,6 +11,12 @@ Security: unaffected. Users who have only used the mosquitto_passwd utility to create and modify their password files are unaffected by this vulnerability. Affects version 1.0 to 1.5.5 inclusive. +- CVE-2018-xxxxx: If an ACL file is empty, or has only blank lines or + comments, then mosquitto treats the ACL file as not being defined, which + means that no topic access is denied. Although denying access to all topics + is not a useful configuration, this behaviour is unexpected and could lead + to access being incorrectly granted in some circumstances. This is now + fixed. Affects versions 1.0 to 1.5.5 inclusive. Broker: - Fixed comment handling for config options that have optional arguments. diff --git a/src/security_default.c b/src/security_default.c index 743020c1a9..5a886a53df 100644 --- a/src/security_default.c +++ b/src/security_default.c @@ -317,7 +317,7 @@ int mosquitto_acl_check_default(struct mosquitto_db *db, struct mosquitto *conte }else{ security_opts = &db->config->security_options; } - if(!security_opts->acl_list && !security_opts->acl_patterns){ + if(!security_opts->acl_file && !security_opts->acl_list && !security_opts->acl_patterns){ return MOSQ_ERR_PLUGIN_DEFER; } @@ -535,6 +535,10 @@ static int aclfile__parse(struct mosquitto_db *db, struct mosquitto__security_op fclose(aclfptr); return 1; } + }else{ + log__printf(NULL, MOSQ_LOG_ERR, "Error: Invalid line in acl_file \"%s\": %s.", security_opts->acl_file, buf); + fclose(aclfptr); + return 1; } } } diff --git a/test/broker/09-acl-empty-file.py b/test/broker/09-acl-empty-file.py new file mode 100755 index 0000000000..a6d71cf60b --- /dev/null +++ b/test/broker/09-acl-empty-file.py @@ -0,0 +1,71 @@ +#!/usr/bin/env python + +# Test for CVE-2018-xxxxx + +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 +import signal + +def write_config(filename, port, per_listener): + with open(filename, 'w') as f: + f.write("per_listener_settings %s\n" % (per_listener)) + f.write("port %d\n" % (port)) + f.write("acl_file %s\n" % (filename.replace('.conf', '.acl'))) + +def write_acl(filename): + with open(filename, 'w') as f: + f.write('#comment\n') + f.write('\n') + + +def do_test(port, per_listener): + conf_file = os.path.basename(__file__).replace('.py', '.conf') + write_config(conf_file, port, per_listener) + + acl_file = os.path.basename(__file__).replace('.py', '.acl') + write_acl(acl_file) + + rc = 1 + keepalive = 60 + connect_packet = mosq_test.gen_connect("acl-check", keepalive=keepalive) + connack_packet = mosq_test.gen_connack(rc=0) + + mid = 1 + publish_packet = mosq_test.gen_publish("test/topic", qos=0, payload="message") + subscribe_packet = mosq_test.gen_subscribe(mid, "test/topic", 0) + suback_packet = mosq_test.gen_suback(mid, 0) + + pingreq_packet = mosq_test.gen_pingreq() + pingresp_packet = mosq_test.gen_pingresp() + + broker = mosq_test.start_broker(filename=os.path.basename(__file__), use_conf=True, port=port) + + try: + sock = mosq_test.do_client_connect(connect_packet, connack_packet, port=port) + mosq_test.do_send_receive(sock, subscribe_packet, suback_packet, "suback") + + sock.send(publish_packet) + + # If we receive the message, this will fail. + mosq_test.do_send_receive(sock, pingreq_packet, pingresp_packet, "pingresp") + rc = 0 + + sock.close() + finally: + os.remove(conf_file) + os.remove(acl_file) + broker.terminate() + broker.wait() + (stdo, stde) = broker.communicate() + if rc: + print(stde) + exit(rc) + +port = mosq_test.get_port() +do_test(port, "false") +do_test(port, "true") diff --git a/test/broker/Makefile b/test/broker/Makefile index 3299d7dbe1..d3b3e43889 100644 --- a/test/broker/Makefile +++ b/test/broker/Makefile @@ -118,6 +118,7 @@ endif endif 09 : + ./09-acl-empty-file.py ./09-plugin-auth-unpwd-success.py ./09-plugin-auth-unpwd-fail.py ./09-plugin-auth-acl-sub.py diff --git a/test/broker/ptest.py b/test/broker/ptest.py index eb10895da5..28f14f97e2 100755 --- a/test/broker/ptest.py +++ b/test/broker/ptest.py @@ -90,6 +90,7 @@ (2, './08-tls-psk-pub.py'), (3, './08-tls-psk-bridge.py'), + (1, './09-acl-empty-file.py'), (1, './09-plugin-auth-unpwd-success.py'), (1, './09-plugin-auth-unpwd-fail.py'), (1, './09-plugin-auth-acl-sub.py'),