Browse Source

mqtt: detect illegal and too large file size

Add test 3017 and 3018 to verify.
Closes #7166
Harry Sintonen 3 years ago
parent
commit
8ccf75532b
5 changed files with 164 additions and 4 deletions
  1. 10 0
      lib/mqtt.c
  2. 3 1
      tests/data/Makefile.inc
  3. 67 0
      tests/data/test3017
  4. 65 0
      tests/data/test3018
  5. 19 3
      tests/server/mqttd.c

+ 10 - 0
lib/mqtt.c

@@ -477,6 +477,12 @@ static CURLcode mqtt_read_publish(struct Curl_easy *data, bool *done)
     /* -- switched state -- */
     remlen = mq->remaining_length;
     infof(data, "Remaining length: %zd bytes\n", remlen);
+    if(data->set.max_filesize &&
+       (curl_off_t)remlen > data->set.max_filesize) {
+      failf(data, "Maximum file size exceeded");
+      result = CURLE_FILESIZE_EXCEEDED;
+      goto end;
+    }
     Curl_pgrsSetDownloadSize(data, remlen);
     data->req.bytecount = 0;
     data->req.size = remlen;
@@ -582,6 +588,10 @@ static CURLcode mqtt_doing(struct Curl_easy *data, bool *done)
       Curl_debug(data, CURLINFO_HEADER_IN, (char *)&byte, 1);
       pkt[mq->npacket++] = byte;
     } while((byte & 0x80) && (mq->npacket < 4));
+    if(nread && (byte & 0x80))
+      /* MQTT supports up to 127 * 128^0 + 127 * 128^1 + 127 * 128^2 +
+         127 * 128^3 bytes. server tried to send more */
+      result = CURLE_WEIRD_SERVER_REPLY;
     if(result)
       break;
     mq->remaining_length = mqtt_decode_len(&pkt[0], mq->npacket, NULL);

+ 3 - 1
tests/data/Makefile.inc

@@ -232,4 +232,6 @@ test2100 \
 \
 test3000 test3001 test3002 test3003 test3004 test3005 test3006 test3007 \
 test3008 test3009 test3010 test3011 test3012 test3013 test3014 test3015 \
-test3016
+test3016 \
+\
+test3017 test3018

+ 67 - 0
tests/data/test3017

@@ -0,0 +1,67 @@
+<testcase>
+<info>
+<keywords>
+MQTT
+MQTT SUBSCRIBE
+</keywords>
+</info>
+
+#
+# Server-side
+<reply>
+<data nocheck="yes">
+hello
+</data>
+<datacheck hex="yes">
+00 04 33 30 31 37   68 65 6c 6c 6f 5b 4c 46 5d 0a
+</datacheck>
+<servercmd>
+excessive-remaining TRUE
+</servercmd>
+</reply>
+
+#
+# Client-side
+<client>
+<features>
+mqtt
+</features>
+<server>
+mqtt
+</server>
+<name>
+MQTT SUBSCRIBE with pathological PUBLISH length
+</name>
+<command option="binary-trace">
+mqtt://%HOSTIP:%MQTTPORT/%TESTNUMBER -m 3
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+# These are hexadecimal protocol dumps from the client
+#
+# Strip out the random part of the client id from the CONNECT message
+# before comparison
+<strippart>
+s/^(.* 00044d5154540402003c000c6375726c).*/$1/
+</strippart>
+# on windows the disconnect is never seen - no idea why
+<strip>
+^server DISCONNECT 0 e000
+</strip>
+<protocol>
+client CONNECT 18 00044d5154540402003c000c6375726c
+server CONNACK 2 20020000
+client SUBSCRIBE 9 000100043330313700
+server SUBACK 3 9003000100
+server PUBLISH c 30ffffff8000043330313768656c6c6f0a
+</protocol>
+
+# 8 is CURLE_WEIRD_SERVER_REPLY
+<errorcode>
+8
+</errorcode>
+</verify>
+</testcase>

+ 65 - 0
tests/data/test3018

@@ -0,0 +1,65 @@
+<testcase>
+<info>
+<keywords>
+MQTT
+MQTT SUBSCRIBE
+--max-filesize
+</keywords>
+</info>
+
+#
+# Server-side
+<reply>
+<data nocheck="yes">
+hello
+</data>
+<datacheck hex="yes">
+00 04 33 30 31 38   68 65 6c 6c 6f 5b 4c 46 5d 0a
+</datacheck>
+</reply>
+
+#
+# Client-side
+<client>
+<features>
+mqtt
+</features>
+<server>
+mqtt
+</server>
+<name>
+MQTT SUBSCRIBE with PUBLISH larger than --max-filesize
+</name>
+<command option="binary-trace">
+mqtt://%HOSTIP:%MQTTPORT/%TESTNUMBER --max-filesize 11
+</command>
+</client>
+
+#
+# Verify data after the test has been "shot"
+<verify>
+# These are hexadecimal protocol dumps from the client
+#
+# Strip out the random part of the client id from the CONNECT message
+# before comparison
+<strippart>
+s/^(.* 00044d5154540402003c000c6375726c).*/$1/
+</strippart>
+# on windows the disconnect is never seen - no idea why
+<strip>
+^server DISCONNECT 0 e000
+</strip>
+<protocol>
+client CONNECT 18 00044d5154540402003c000c6375726c
+server CONNACK 2 20020000
+client SUBSCRIBE 9 000100043330313800
+server SUBACK 3 9003000100
+server PUBLISH c 300c00043330313868656c6c6f0a
+</protocol>
+
+# 63 is CURLE_FILESIZE_EXCEEDED
+<errorcode>
+63
+</errorcode>
+</verify>
+</testcase>

+ 19 - 3
tests/server/mqttd.c

@@ -106,6 +106,7 @@ struct configurable {
                             this */
   bool publish_before_suback;
   bool short_publish;
+  bool excessive_remaining;
   unsigned char error_connack;
   int testnum;
 };
@@ -130,6 +131,7 @@ static void resetdefaults(void)
   config.version = CONFIG_VERSION;
   config.publish_before_suback = FALSE;
   config.short_publish = FALSE;
+  config.excessive_remaining = FALSE;
   config.error_connack = 0;
   config.testnum = 0;
 }
@@ -171,6 +173,10 @@ static void getconfig(void)
           config.testnum = atoi(value);
           logmsg("testnum = %d", config.testnum);
         }
+        else if(!strcmp(key, "excessive-remaining")) {
+          logmsg("excessive-remaining set");
+          config.excessive_remaining = TRUE;
+        }
       }
     }
     fclose(fp);
@@ -337,7 +343,8 @@ static int disconnect(FILE *dump, curl_socket_t fd)
 */
 
 /* return number of bytes used */
-static int encode_length(size_t packetlen, char *remlength) /* 4 bytes */
+static int encode_length(size_t packetlen,
+                         unsigned char *remlength) /* 4 bytes */
 {
   int bytes = 0;
   unsigned char encode;
@@ -393,10 +400,19 @@ static int publish(FILE *dump,
   ssize_t packetlen;
   ssize_t sendamount;
   ssize_t rc;
-  char rembuffer[4];
+  unsigned char rembuffer[4];
   int encodedlen;
 
-  encodedlen = encode_length(remaininglength, rembuffer);
+  if(config.excessive_remaining) {
+    /* manually set illegal remaining length */
+    rembuffer[0] = 0xff;
+    rembuffer[1] = 0xff;
+    rembuffer[2] = 0xff;
+    rembuffer[3] = 0x80; /* maximum allowed here by spec is 0x7f */
+    encodedlen = 4;
+  }
+  else
+    encodedlen = encode_length(remaininglength, rembuffer);
 
   /* one packet type byte (possibly two more for packetid) */
   packetlen = remaininglength + encodedlen + 1;