Fix 5 bugs: asn1_encode threshold, MQTT bounds/mid, unpack underflow, null deref

base/hmath.h

@@ -92,7 +92,7 @@ static inline int asn1_encode(long long value, unsigned char* buf) {
         *p = (unsigned char)value;
         return 3;
     }
-    else if (value < 16777126)
+    else if (value < 16777216)
     {
         *p = 0x83;
         p++;

event/unpack.c

@@ -157,7 +157,20 @@ int hio_unpack_by_length_field(hio_t* io, void* buf, int readbytes) {
             hio_close(io);
             return -1;
         }
-        package_len = head_len + body_len + setting->length_adjustment;
+        long long signed_package_len = (long long)head_len + (long long)body_len + setting->length_adjustment;
+        if (signed_package_len <= 0) {
+            hloge("Invalid package length %lld!", signed_package_len);
+            io->error = ERR_INVALID_PACKAGE;
+            hio_close(io);
+            return -1;
+        }
+        if (signed_package_len > setting->package_max_length) {
+            hloge("Package length over %u bytes!", setting->package_max_length);
+            io->error = ERR_OVER_LIMIT;
+            hio_close(io);
+            return -1;
+        }
+        package_len = (unsigned int)signed_package_len;
         if (remain >= package_len) {
             hio_read_cb(io, (void*)p, package_len);
             handled += package_len;

mqtt/mqtt_client.c

@@ -6,9 +6,12 @@
 #include "hsocket.h"
 #include "hmath.h"
 
-static unsigned short mqtt_next_mid() {
-    static unsigned short s_mid = 0;
-    return ++s_mid;
+static unsigned short mqtt_next_mid(mqtt_client_t* cli) {
+    hmutex_lock(&cli->mutex_);
+    if (++cli->mid_ == 0) cli->mid_ = 1;
+    unsigned short mid = cli->mid_;
+    hmutex_unlock(&cli->mutex_);
+    return mid;
 }
 
 static int mqtt_client_send(mqtt_client_t* cli, const void* buf, int len) {
@@ -231,6 +234,7 @@ static void mqtt_client_add_reconnect_timer(mqtt_client_t* cli) {
 
 static void on_close(hio_t* io) {
     mqtt_client_t* cli = (mqtt_client_t*)hevent_userdata(io);
+    if (cli == NULL) return;
     cli->connected = 0;
     if (cli->cb) {
         cli->head.type = MQTT_TYPE_DISCONNECT;
@@ -627,7 +631,7 @@ int mqtt_client_publish(mqtt_client_t* cli, mqtt_message_t* msg) {
     PUSH16(p, topic_len);
     PUSH_N(p, msg->topic, topic_len);
     if (msg->qos) {
-        mid = mqtt_next_mid();
+        mid = mqtt_next_mid(cli);
         PUSH16(p, mid);
     }
     // MQTT v5: publish properties (empty)
@@ -670,7 +674,7 @@ int mqtt_client_subscribe(mqtt_client_t* cli, const char* topic, int qos) {
     unsigned char* p = buf;
     int headlen = mqtt_head_pack(&head, p);
     p += headlen;
-    unsigned short mid = mqtt_next_mid();
+    unsigned short mid = mqtt_next_mid(cli);
     PUSH16(p, mid);
     // MQTT v5: subscribe properties (empty)
     if (cli->protocol_version == MQTT_PROTOCOL_V5) {
@@ -704,7 +708,7 @@ int mqtt_client_unsubscribe(mqtt_client_t* cli, const char* topic) {
     unsigned char* p = buf;
     int headlen = mqtt_head_pack(&head, p);
     p += headlen;
-    unsigned short mid = mqtt_next_mid();
+    unsigned short mid = mqtt_next_mid(cli);
     PUSH16(p, mid);
     // MQTT v5: unsubscribe properties (empty)
     if (cli->protocol_version == MQTT_PROTOCOL_V5) {

mqtt/mqtt_client.h

@@ -53,6 +53,7 @@ struct mqtt_client_s {
     hssl_ctx_t ssl_ctx;
     // thread-safe
     hmutex_t mutex_;
+    unsigned short mid_;  // message ID counter, protected by mutex_
 };
 
 BEGIN_EXTERN_C

mqtt/mqtt_protocol.c

@@ -11,6 +11,7 @@ int mqtt_head_pack(mqtt_head_t* head, unsigned char buf[]) {
 }
 
 int mqtt_head_unpack(mqtt_head_t* head, const unsigned char* buf, int len) {
+    if (len < 2) return 0;
     head->type   = (buf[0] >> 4) & 0x0F;
     head->dup    = (buf[0] >> 3) & 0x01;
     head->qos    = (buf[0] >> 1) & 0x03;