From 1ef15a2f99af3e37b8940238e60bfd512cd67c57 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 06:49:19 +0000 Subject: [PATCH 1/4] Initial plan From d709bc051de256d955c71ee1b0ecd15ac70804d6 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 06:54:41 +0000 Subject: [PATCH 2/4] Fix potential bugs: asn1_encode constant, mqtt_head_unpack bounds check, mqtt_next_mid wrap, unpack underflow, on_close null check Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com> --- base/hmath.h | 2 +- event/unpack.c | 9 ++++++++- mqtt/mqtt_client.c | 4 +++- mqtt/mqtt_protocol.c | 1 + 4 files changed, 13 insertions(+), 3 deletions(-) diff --git a/base/hmath.h b/base/hmath.h index 6cff816f7..27570911a 100644 --- a/base/hmath.h +++ b/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++; diff --git a/event/unpack.c b/event/unpack.c index 604b53c91..535a72c2f 100644 --- a/event/unpack.c +++ b/event/unpack.c @@ -157,7 +157,14 @@ 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; + int signed_package_len = (int)head_len + (int)body_len + setting->length_adjustment; + if (signed_package_len <= 0 || signed_package_len > (int)setting->package_max_length) { + hloge("Invalid package length %d!", signed_package_len); + 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; diff --git a/mqtt/mqtt_client.c b/mqtt/mqtt_client.c index 9e4566d34..a02886230 100644 --- a/mqtt/mqtt_client.c +++ b/mqtt/mqtt_client.c @@ -8,7 +8,8 @@ static unsigned short mqtt_next_mid() { static unsigned short s_mid = 0; - return ++s_mid; + if (++s_mid == 0) s_mid = 1; + return s_mid; } static int mqtt_client_send(mqtt_client_t* cli, const void* buf, int len) { @@ -231,6 +232,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; diff --git a/mqtt/mqtt_protocol.c b/mqtt/mqtt_protocol.c index f55609793..3fcb79da8 100644 --- a/mqtt/mqtt_protocol.c +++ b/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; From 2bdf6f1c539d8ff9b53e05a49838058847d26dce Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 07:19:18 +0000 Subject: [PATCH 3/4] Address review: use long long for package length, make mqtt_next_mid per-client and thread-safe Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com> --- event/unpack.c | 12 +++++++++--- mqtt/mqtt_client.c | 16 +++++++++------- mqtt/mqtt_client.h | 1 + 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/event/unpack.c b/event/unpack.c index 535a72c2f..efc604e17 100644 --- a/event/unpack.c +++ b/event/unpack.c @@ -157,9 +157,15 @@ int hio_unpack_by_length_field(hio_t* io, void* buf, int readbytes) { hio_close(io); return -1; } - int signed_package_len = (int)head_len + (int)body_len + setting->length_adjustment; - if (signed_package_len <= 0 || signed_package_len > (int)setting->package_max_length) { - hloge("Invalid package length %d!", signed_package_len); + 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; diff --git a/mqtt/mqtt_client.c b/mqtt/mqtt_client.c index a02886230..a0f72f088 100644 --- a/mqtt/mqtt_client.c +++ b/mqtt/mqtt_client.c @@ -6,10 +6,12 @@ #include "hsocket.h" #include "hmath.h" -static unsigned short mqtt_next_mid() { - static unsigned short s_mid = 0; - if (++s_mid == 0) s_mid = 1; - 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) { @@ -629,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) @@ -672,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) { @@ -706,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) { diff --git a/mqtt/mqtt_client.h b/mqtt/mqtt_client.h index 4211ddbc8..fb479ebd7 100644 --- a/mqtt/mqtt_client.h +++ b/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_; }; BEGIN_EXTERN_C From 3e30f130837709a488bf1e293bd60e1ad87c98db Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 07:20:16 +0000 Subject: [PATCH 4/4] Add documentation comment for mid_ field in mqtt_client_s Co-authored-by: ithewei <26049660+ithewei@users.noreply.github.com> --- mqtt/mqtt_client.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mqtt/mqtt_client.h b/mqtt/mqtt_client.h index fb479ebd7..a58a09409 100644 --- a/mqtt/mqtt_client.h +++ b/mqtt/mqtt_client.h @@ -53,7 +53,7 @@ struct mqtt_client_s { hssl_ctx_t ssl_ctx; // thread-safe hmutex_t mutex_; - unsigned short mid_; + unsigned short mid_; // message ID counter, protected by mutex_ }; BEGIN_EXTERN_C