reading mosquitto.conf with line > 1023 chars does unexpected things (with patch)

[edwin-oetelaar]

while testing TLS I added a line :

ciphers ECDHE-RSA-AES256-GCM-SHA384:ECDHE-E.......

The line is 1600 chars long, the config reader fails unexpectedly :

Error: Unable to open configuration file.
Error: Unknown configuration variable ":DHE-RSA-CAMELLIA128-SHA:DHE-DSS-...

It also fails if the ciphers line starts with # and is commented out

Any line longer than 1023 will be read as a new configuration line starting at byte 1024... this has unexpected consequenses. (reading commented content as if it had not been commented out, possible security implications? )

The problem is in src/conf.c line 572

mosquitto/src/conf.c

Line 572 in 46630e7

while(fgets(buf, 1024, fptr)){

the buffer is on the stack:

char buf[1024];

mosquitto/src/conf.c

Line 543 in 46630e7

char buf[1024];

I could fix this and submit a patch if you like that. (please say so)

For my test setup I will just hack the buffer to 4096 bytes for now and continue testing.

[toast-uz]

You are right. But try "openssl ciphers shortening strings" may minimize the length of the list of valid ciphers. See mosquitto.conf and https://www.openssl.org/docs/man1.0.2/apps/ciphers.html for details.

[edwin-oetelaar]

I think 3 things should be done.

• 1: make the buffer 2048 bytes instead of 1024 bytes so long VALID lines will work without problem
• 2: make reading more robust so very long COMMENT lines will not break the config reader
• 3: stop reading config file and program termination on invalid lines in config file

[edwin-oetelaar]

Here is the fix (fully tested), but I can make a pull request if you want that.

Best regards,
Edwin van den Oetelaar

diff` --git a/src/conf.c b/src/conf.c
index a3e233d..452608b 100644
--- a/src/conf.c
+++ b/src/conf.c
@@ -540,7 +540,8 @@ int mqtt3_config_read(struct mqtt3_config *config, bool reload)
 int _config_read_file_core(struct mqtt3_config *config, bool reload, const char *file, struct config_recurse *cr, int level, int *lineno, FILE *fptr)
 {
        int rc;
-       char buf[1024];
+    static const uint32_t bufsize = 2048; /* line buffer size */
+       char buf[bufsize]; /* line buffer */
        char *token;
        int tmp_int;
        char *saveptr = NULL;
@@ -569,8 +570,12 @@ int _config_read_file_core(struct mqtt3_config *config, bool reload, const char
 
        *lineno = 0;
 
-       while(fgets(buf, 1024, fptr)){
+       while(fgets(buf, bufsize, fptr)){
                (*lineno)++;
+        if(strlen(buf) == bufsize-1){
+            _mosquitto_log_printf(NULL, MOSQ_LOG_ERR, "Error: Line too long in configuration.");
+            return MOSQ_ERR_INVAL;
+        }
                if(buf[0] != '#' && buf[0] != 10 && buf[0] != 13){
                        while(buf[strlen(buf)-1] == 10 || buf[strlen(buf)-1] == 13){
                                buf[strlen(buf)-1] = 0;

[ralight]

Thanks for the report and the patch, I've fixed it in a slightly different way that means it should never be a problem again.