From 99dc10fc0c59d73b7f6845d8acf2874fb617cd1e Mon Sep 17 00:00:00 2001
From: Eric Blankenhorn <eric@wolfssl.com>
Date: Mon, 6 Jul 2026 10:05:18 -0500
Subject: [PATCH 1/5] Fix f-3627: test BrokerBufCompare length-fold auth
 backstop

---
 tests/test_broker_connect.c | 65 +++++++++++++++++++++++++++++++++++--
 1 file changed, 62 insertions(+), 3 deletions(-)

diff --git a/tests/test_broker_connect.c b/tests/test_broker_connect.c
index 5f72df10a..5bf10e5ff 100644
--- a/tests/test_broker_connect.c
+++ b/tests/test_broker_connect.c
@@ -539,7 +539,7 @@ TEST(connect_v311_binary_password_with_embedded_nul_refused)
     reset_mock_state(connect, sizeof(connect));
     run_broker_one_connect(&broker);
 
-    /* Auth must fail - CONNACK return code 0x05 (Not Authorized) and the
+    /* Auth must fail - CONNACK return code 0x04 (Bad user/pass) and the
      * connection is closed. Pre-fix, XSTRLEN truncation would let this
      * authenticate and emit return code 0x00. */
     ASSERT_TRUE(g_out_len >= 4);
@@ -590,6 +590,64 @@ TEST(connect_v311_binary_password_exact_match_accepted)
     MqttBroker_Free(&broker);
 }
 
+/* Mutation guard for the length-fold backstop in BrokerBufCompare (the
+ * final "result |= (len_a ^ len_b)"). The per-byte constant-time loop
+ * clamps an out-of-range index to position 0, so it is blind to a length
+ * mismatch whenever the shorter input's bytes repeat through the longer
+ * one: comparing configured "aaaaa" against a supplied "a" sees only
+ * 'a' ^ 'a' == 0 at every clamped position, and only the length fold
+ * rejects it. Every other negative auth test differs in actual byte
+ * content within range, so the byte loop alone rejects those and none of
+ * them exercise this line; deleting the fold would let a configured user
+ * "aaaaa" be authenticated by the username "a".
+ *
+ * The username here is the repeating-byte length case while the password
+ * matches auth_pass exactly, so the password compare is neutral and the
+ * username length fold alone decides the outcome. Auth must fail (CONNACK
+ * 0x04 Bad user/pass, connection closed). With the fold deleted this would
+ * accept (return code 0x00) - the mutation-detecting assertion. */
+TEST(connect_auth_username_length_fold_repeating_byte_refused)
+{
+    MqttBroker broker;
+    MqttBrokerNet net;
+    /* CONNECT v3.1.1, ClientId "id", Username "a" (len 1), Password
+     * "aaaaa" (len 5, exact match for auth_pass). connect_flags 0xC2 =
+     * username + password + clean session. */
+    static const byte connect[] = {
+        0x10, 24,                                  /* fixed header, rl 24 */
+        0x00, 0x04, 'M', 'Q', 'T', 'T',            /* protocol name */
+        0x04,                                      /* level 4 (v3.1.1) */
+        0xC2,                                      /* flags: user+pass+clean */
+        0x00, 0x3C,                                /* keep-alive 60 */
+        0x00, 0x02, 'i', 'd',                      /* ClientId "id" */
+        0x00, 0x01, 'a',                           /* Username "a" */
+        0x00, 0x05, 'a', 'a', 'a', 'a', 'a'        /* Password "aaaaa" */
+    };
+
+    install_mock_net(&net);
+    XMEMSET(&broker, 0, sizeof(broker));
+    ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net));
+    broker.auth_user = "aaaaa";
+    broker.auth_pass = "aaaaa";
+    ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker));
+
+    reset_mock_state(connect, sizeof(connect));
+    run_broker_one_connect(&broker);
+
+    /* Auth must fail - CONNACK return code 0x04 (Bad user/pass) and the
+     * connection closed. Deleting the length fold would authenticate the
+     * shorter repeating-byte username and emit return code 0x00. */
+    ASSERT_TRUE(g_out_len >= 4);
+    ASSERT_EQ(0x20, g_out_buf[0]);
+    ASSERT_EQ(0x02, g_out_buf[1]);
+    ASSERT_EQ(0x00, g_out_buf[2]);
+    ASSERT_EQ(MQTT_CONNECT_ACK_CODE_REFUSED_BAD_USER_PWD, g_out_buf[3]);
+    ASSERT_TRUE(g_client_closed);
+
+    MqttBroker_Stop(&broker);
+    MqttBroker_Free(&broker);
+}
+
 /* An unauthenticated CONNECT must not mutate another
  * client's session. A victim authenticates and stays connected; an attacker
  * then reuses the victim's client_id with a wrong password. The broker must
@@ -717,7 +775,7 @@ TEST(connect_auth_partial_config_fails_closed)
 
     /* Pre-fix the username matched and the absent password was never
      * checked, so the broker accepted (return code 0x00). The gate must now
-     * refuse with 0x05 (Bad user/pass) and close the connection. */
+     * refuse with 0x04 (Bad user/pass) and close the connection. */
     ASSERT_TRUE(g_out_len >= 4);
     ASSERT_EQ(0x20, g_out_buf[0]);
     ASSERT_EQ(0x02, g_out_buf[1]);
@@ -761,7 +819,7 @@ TEST(connect_auth_partial_pass_only_fails_closed)
     run_broker_one_connect(&broker);
 
     /* Pre-fix the password matched and the username was never checked, so the
-     * broker accepted (return code 0x00). The gate must now refuse with 0x05
+     * broker accepted (return code 0x00). The gate must now refuse with 0x04
      * (Bad user/pass) and close the connection. */
     ASSERT_TRUE(g_out_len >= 4);
     ASSERT_EQ(0x20, g_out_buf[0]);
@@ -3201,6 +3259,7 @@ int main(int argc, char** argv)
 #ifdef WOLFMQTT_BROKER_AUTH
     RUN_TEST(connect_v311_binary_password_with_embedded_nul_refused);
     RUN_TEST(connect_v311_binary_password_exact_match_accepted);
+    RUN_TEST(connect_auth_username_length_fold_repeating_byte_refused);
     RUN_TEST(connect_unauth_client_id_does_not_take_over_victim);
     RUN_TEST(connect_auth_user_only_start_rejected);
     RUN_TEST(connect_auth_pass_only_start_rejected);

From c4126eed655e81a92a5274f5691f7ed513370345 Mon Sep 17 00:00:00 2001
From: Eric Blankenhorn <eric@wolfssl.com>
Date: Mon, 6 Jul 2026 11:58:44 -0500
Subject: [PATCH 2/5] Fix f-2359: preserve v5 SUBSCRIBE options through
 encode/decode

---
 examples/mqttsimple/mqttsimple.c      |   1 +
 examples/websocket/websocket_client.c |   1 +
 src/mqtt_packet.c                     |  19 ++++-
 tests/test_mqtt_packet.c              | 102 ++++++++++++++++++++++++++
 wolfmqtt/mqtt_packet.h                |  20 +++++
 5 files changed, 142 insertions(+), 1 deletion(-)

diff --git a/examples/mqttsimple/mqttsimple.c b/examples/mqttsimple/mqttsimple.c
index ef23c0852..fd1bf541a 100644
--- a/examples/mqttsimple/mqttsimple.c
+++ b/examples/mqttsimple/mqttsimple.c
@@ -411,6 +411,7 @@ int mqttsimple_test(void)
 
     /* Subscribe and wait for Ack */
     XMEMSET(&mqttObj, 0, sizeof(mqttObj));
+    XMEMSET(topics, 0, sizeof(topics));
     topics[0].topic_filter = MQTT_TOPIC_NAME;
     topics[0].qos = MQTT_QOS;
     mqttObj.subscribe.packet_id = mqtt_get_packetid();
diff --git a/examples/websocket/websocket_client.c b/examples/websocket/websocket_client.c
index 3db8d463b..c7bd059fa 100644
--- a/examples/websocket/websocket_client.c
+++ b/examples/websocket/websocket_client.c
@@ -181,6 +181,7 @@ int main(int argc, char *argv[])
     MqttSubscribe subscribe;
     MqttTopic topics[1];
     XMEMSET(&subscribe, 0, sizeof(subscribe));
+    XMEMSET(topics, 0, sizeof(topics));
     topics[0].topic_filter = "test/topic";
     topics[0].qos = MQTT_QOS_0;
     subscribe.packet_id = mqtt_get_packetid();
diff --git a/src/mqtt_packet.c b/src/mqtt_packet.c
index 8b5ea3156..93f717552 100644
--- a/src/mqtt_packet.c
+++ b/src/mqtt_packet.c
@@ -2567,11 +2567,21 @@ int MqttEncode_Subscribe(byte *tx_buf, int tx_buf_len,
 
     /* Encode payload */
     for (i = 0; i < subscribe->topic_count; i++) {
+        byte options;
         topic = &subscribe->topics[i];
         tx_payload += MqttEncode_String(tx_payload, topic->topic_filter);
+        options = (byte)topic->qos;
+    #ifdef WOLFMQTT_V5
+        /* [MQTT v5 3.8.3.1] merge No Local, Retain As Published and Retain
+         * Handling (bits 2-5). These bits are reserved (MUST be 0) in
+         * v3.1.1, so only fold them in for a v5 SUBSCRIBE. */
+        if (subscribe->protocol_level >= MQTT_CONNECT_PROTOCOL_LEVEL_5) {
+            options |= (byte)(topic->sub_options & MQTT_SUBSCRIBE_OPTIONS_MASK);
+        }
+    #endif
         /* Sanity check for compilers */
         if (tx_payload != NULL) {
-            *tx_payload = topic->qos;
+            *tx_payload = options;
         }
         tx_payload++;
     }
@@ -2713,6 +2723,13 @@ int MqttDecode_Subscribe(byte *rx_buf, int rx_buf_len, MqttSubscribe *subscribe)
                 }
             }
             topic->qos = (MqttQoS)(options & 0x03);
+        #ifdef WOLFMQTT_V5
+            /* Preserve v5 No Local / Retain As Published / Retain Handling
+             * (bits 2-5) so the full options byte round-trips. Reserved and
+             * QoS bits were validated above; for a valid v3.1.1 packet these
+             * bits are 0. */
+            topic->sub_options = (byte)(options & MQTT_SUBSCRIBE_OPTIONS_MASK);
+        #endif
             subscribe->topic_count++;
         }
 
diff --git a/tests/test_mqtt_packet.c b/tests/test_mqtt_packet.c
index ff63f828f..b8c0c60b6 100644
--- a/tests/test_mqtt_packet.c
+++ b/tests/test_mqtt_packet.c
@@ -1849,6 +1849,39 @@ TEST(encode_subscribe_options_byte_qos2)
     ASSERT_EQ(0x02, tx_buf[rc - 1]);
 }
 
+#ifdef WOLFMQTT_V5
+/* f-2359: the v5 option bits (No Local / Retain As Published / Retain
+ * Handling) MUST NOT appear on a v3.1.1 SUBSCRIBE, where bits 2-7 of the
+ * options byte are reserved and MUST be 0. With protocol_level < 5 the
+ * encoder must ignore sub_options and emit a QoS-only options byte. Locks
+ * the protocol_level guard so a refactor that always folded sub_options
+ * (emitting reserved bits on a 3.1.1 wire) can't pass CI. */
+TEST(encode_subscribe_v311_ignores_sub_options)
+{
+    byte tx_buf[256];
+    MqttSubscribe sub;
+    MqttTopic topic;
+    int rc;
+
+    XMEMSET(&sub, 0, sizeof(sub));
+    XMEMSET(&topic, 0, sizeof(topic));
+    topic.topic_filter = "a";
+    topic.qos = MQTT_QOS_1;
+    /* Request every v5 option bit; a v3.1.1 encode must drop them all. */
+    topic.sub_options = MQTT_SUBSCRIBE_NO_LOCAL |
+                        MQTT_SUBSCRIBE_RETAIN_AS_PUBLISHED |
+                        MQTT_SUBSCRIBE_RETAIN_HANDLING_2;
+    sub.topics = &topic;
+    sub.topic_count = 1;
+    sub.packet_id = 1;
+    sub.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_4;
+    rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub);
+    ASSERT_TRUE(rc > 0);
+    /* Options byte is the last byte: QoS 1 only, no v5 option bits. */
+    ASSERT_EQ(0x01, tx_buf[rc - 1]);
+}
+#endif /* WOLFMQTT_V5 */
+
 /* f-2360: topic_filter with strlen > 65535 must be rejected with a negative
  * return. Guards the unchecked tx_payload += MqttEncode_String(...) in the
  * SUBSCRIBE payload loop. */
@@ -3485,6 +3518,71 @@ TEST(decode_subscribe_v5_options_reserved_bits_rejected)
     rc = MqttDecode_Subscribe(rx_buf, (int)sizeof(rx_buf), &sub);
     ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc);
 }
+
+/* f-2359: the v5 SUBSCRIBE options byte carries No Local (bit 2), Retain As
+ * Published (bit 3) and Retain Handling (bits 4-5) alongside QoS (bits 0-1).
+ * Encode then decode must preserve the full options byte via
+ * MqttTopic.sub_options, not just QoS. Exhaustively covers every valid
+ * combination: QoS 0-2 x No Local x RAP x Retain Handling 0-2 (36 cases). */
+TEST(subscribe_v5_options_roundtrip_all_combos)
+{
+    static const byte rh_vals[] = {
+        MQTT_SUBSCRIBE_RETAIN_HANDLING_0,
+        MQTT_SUBSCRIBE_RETAIN_HANDLING_1,
+        MQTT_SUBSCRIBE_RETAIN_HANDLING_2
+    };
+    byte tx_buf[64];
+    MqttSubscribe sub;
+    MqttTopic topic;
+    MqttSubscribe dec;
+    MqttTopic dec_topic[1];
+    int qos, nl, rap, rh_i;
+    int enc_len, dec_len;
+    byte want_opts;
+
+    for (qos = MQTT_QOS_0; qos <= MQTT_QOS_2; qos++) {
+        for (nl = 0; nl <= 1; nl++) {
+            for (rap = 0; rap <= 1; rap++) {
+                for (rh_i = 0;
+                     rh_i < (int)(sizeof(rh_vals) / sizeof(rh_vals[0]));
+                     rh_i++) {
+                    want_opts = (byte)(
+                        (nl  ? MQTT_SUBSCRIBE_NO_LOCAL : 0) |
+                        (rap ? MQTT_SUBSCRIBE_RETAIN_AS_PUBLISHED : 0) |
+                        rh_vals[rh_i]);
+
+                    XMEMSET(&sub, 0, sizeof(sub));
+                    XMEMSET(&topic, 0, sizeof(topic));
+                    topic.topic_filter = "a";
+                    topic.qos = (MqttQoS)qos;
+                    topic.sub_options = want_opts;
+                    sub.topics = &topic;
+                    sub.topic_count = 1;
+                    sub.packet_id = 1;
+                    sub.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5;
+
+                    enc_len = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf),
+                        &sub);
+                    ASSERT_TRUE(enc_len > 0);
+                    /* Last byte is the options byte: QoS (bits 0-1) plus the
+                     * v5 option bits 2-5. */
+                    ASSERT_EQ((int)((byte)qos | want_opts),
+                        (int)tx_buf[enc_len - 1]);
+
+                    XMEMSET(&dec, 0, sizeof(dec));
+                    XMEMSET(dec_topic, 0, sizeof(dec_topic));
+                    dec.topics = dec_topic;
+                    dec.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5;
+                    dec_len = MqttDecode_Subscribe(tx_buf, enc_len, &dec);
+                    ASSERT_TRUE(dec_len > 0);
+                    ASSERT_EQ(1, dec.topic_count);
+                    ASSERT_EQ((int)qos, (int)dec_topic[0].qos);
+                    ASSERT_EQ((int)want_opts, (int)dec_topic[0].sub_options);
+                }
+            }
+        }
+    }
+}
 #endif /* WOLFMQTT_V5 */
 
 /* ============================================================================
@@ -4969,6 +5067,9 @@ void run_mqtt_packet_tests(void)
     RUN_TEST(encode_subscribe_options_byte_qos0);
     RUN_TEST(encode_subscribe_options_byte_qos1);
     RUN_TEST(encode_subscribe_options_byte_qos2);
+#ifdef WOLFMQTT_V5
+    RUN_TEST(encode_subscribe_v311_ignores_sub_options);
+#endif
     RUN_TEST(encode_subscribe_topic_filter_oversized_rejected);
     RUN_TEST(encode_subscribe_topic_filter_oversized_second_rejected);
     RUN_TEST(encode_subscribe_has_qos1_flag);
@@ -5050,6 +5151,7 @@ void run_mqtt_packet_tests(void)
     RUN_TEST(decode_subscribe_v5_qos3_rejected);
     RUN_TEST(decode_subscribe_v5_retain_handling_3_rejected);
     RUN_TEST(decode_subscribe_v5_options_reserved_bits_rejected);
+    RUN_TEST(subscribe_v5_options_roundtrip_all_combos);
 #endif
 
     /* MqttDecode_Unsubscribe */
diff --git a/wolfmqtt/mqtt_packet.h b/wolfmqtt/mqtt_packet.h
index 04452d967..4a55130f7 100644
--- a/wolfmqtt/mqtt_packet.h
+++ b/wolfmqtt/mqtt_packet.h
@@ -208,6 +208,21 @@ typedef enum _MqttQoS {
 #endif
 
 
+#ifdef WOLFMQTT_V5
+/* MQTT v5 SUBSCRIBE options byte (section 3.8.3.1), stored in
+ * MqttTopic.sub_options in their on-the-wire bit positions. QoS (bits 0-1)
+ * is carried separately in MqttTopic.qos; bits 6-7 are reserved (MUST be 0).
+ * The encoder trusts the caller and emits sub_options verbatim (masked to
+ * bits 2-5), so supply only spec-valid combinations - in particular Retain
+ * Handling must be 0-2 (the value 3 is reserved and a broker will reject it). */
+#define MQTT_SUBSCRIBE_NO_LOCAL             0x04 /* bit 2 */
+#define MQTT_SUBSCRIBE_RETAIN_AS_PUBLISHED  0x08 /* bit 3 */
+#define MQTT_SUBSCRIBE_RETAIN_HANDLING_0    0x00 /* bits 4-5: send at subscribe */
+#define MQTT_SUBSCRIBE_RETAIN_HANDLING_1    0x10 /* send at subscribe if new */
+#define MQTT_SUBSCRIBE_RETAIN_HANDLING_2    0x20 /* do not send at subscribe */
+#define MQTT_SUBSCRIBE_OPTIONS_MASK         0x3C /* bits 2-5 */
+#endif
+
 /* Topic */
 typedef struct _MqttTopic {
     const char* topic_filter;
@@ -217,6 +232,11 @@ typedef struct _MqttTopic {
     byte        return_code; /* MqttSubscribeAckReturnCodes */
 #ifdef WOLFMQTT_V5
     word16      alias;
+    byte        sub_options; /* v5 SUBSCRIBE options bits 2-5 (No Local,
+                              * Retain As Published, Retain Handling); see
+                              * MQTT_SUBSCRIBE_*. Zero-initialize MqttTopic so
+                              * unset options default to 0 - the v5 encoder now
+                              * reads this field. */
 #endif
 } MqttTopic;
 

From 3bab2b7dd69037762d374765e2141015c83ec512 Mon Sep 17 00:00:00 2001
From: Eric Blankenhorn <eric@wolfssl.com>
Date: Mon, 6 Jul 2026 13:01:08 -0500
Subject: [PATCH 3/5] Fix Fix f-3394: scrub CONNECT credentials from broker
 rx_buf after auth

---
 src/mqtt_broker.c           | 20 +++++++++
 tests/test_broker_connect.c | 87 +++++++++++++++++++++++++++++++++++++
 2 files changed, 107 insertions(+)

diff --git a/src/mqtt_broker.c b/src/mqtt_broker.c
index 9f555ef32..898455b84 100644
--- a/src/mqtt_broker.c
+++ b/src/mqtt_broker.c
@@ -4967,6 +4967,26 @@ static int BrokerHandle_Connect(BrokerClient* bc, int rx_len,
     }
 #endif
 
+    /* CONNECT accepted. Authentication is complete, and neither the plaintext
+     * CONNECT still in bc->rx_buf (mc.username/mc.password were in-place
+     * pointers into it) nor the dedicated bc->password copy is read again for
+     * the life of this connection - the broker has no re-auth path. Scrub both
+     * now so credentials reside in memory only for the duration of CONNECT
+     * processing rather than the whole session. Refused/failed CONNECTs return
+     * earlier and are wiped when the caller frees the client. Mirrors the
+     * client-side CLIENT_FORCE_ZERO hardening in mqtt_client.c. */
+    BROKER_FORCE_ZERO(bc->rx_buf, (word32)rx_len);
+#ifdef WOLFMQTT_BROKER_AUTH
+#ifdef WOLFMQTT_STATIC_MEMORY
+    BROKER_FORCE_ZERO(bc->password, BROKER_MAX_PASSWORD_LEN);
+#else
+    if (bc->password != NULL) {
+        BROKER_FORCE_ZERO(bc->password, (size_t)bc->password_len + 1);
+    }
+#endif
+    bc->password_len = 0;
+#endif
+
     return rc;
 }
 
diff --git a/tests/test_broker_connect.c b/tests/test_broker_connect.c
index 5bf10e5ff..e7ae899c5 100644
--- a/tests/test_broker_connect.c
+++ b/tests/test_broker_connect.c
@@ -986,6 +986,90 @@ static BrokerClient* find_broker_client(MqttBroker* broker, const char* id)
     return NULL;
 }
 
+#ifdef WOLFMQTT_BROKER_AUTH
+/* Return 1 if the byte sequence `needle` (length nlen) appears anywhere in
+ * the first hlen bytes of `hay`, else 0. Used to prove credential plaintext
+ * has been scrubbed from a buffer. */
+static int region_contains(const byte* hay, int hlen,
+    const char* needle, int nlen)
+{
+    int i;
+    if (hay == NULL || nlen <= 0 || hlen < nlen) {
+        return 0;
+    }
+    for (i = 0; i + nlen <= hlen; i++) {
+        if (XMEMCMP(hay + i, needle, (size_t)nlen) == 0) {
+            return 1;
+        }
+    }
+    return 0;
+}
+
+/* f-3394: after an accepted CONNECT the plaintext credentials must not
+ * linger for the connection's lifetime. The decoder leaves mc.username /
+ * mc.password as in-place pointers into bc->rx_buf, and the broker copies the
+ * password into bc->password only for the one-time auth compare. Neither is
+ * read again (the broker has no re-auth path), so BrokerHandle_Connect scrubs
+ * bc->rx_buf and bc->password before returning on the accepted path. Verify
+ * the password plaintext is gone from rx_buf and bc->password_len is cleared
+ * after a successful auth. Dynamic-memory only (uses find_broker_client). */
+TEST(connect_credentials_scrubbed_after_accept)
+{
+    MqttBroker broker;
+    MqttBrokerNet net;
+    BrokerClient* bc;
+    /* CONNECT v3.1.1, ClientId "id", Username "alice", Password "s3cr3tPW".
+     * connect_flags 0xC2 = username + password + clean session. */
+    static const byte connect[] = {
+        0x10, 31,
+        0x00, 0x04, 'M', 'Q', 'T', 'T',
+        0x04,
+        0xC2,
+        0x00, 0x3C,
+        0x00, 0x02, 'i', 'd',
+        0x00, 0x05, 'a', 'l', 'i', 'c', 'e',
+        0x00, 0x08, 's', '3', 'c', 'r', '3', 't', 'P', 'W'
+    };
+
+    install_mock_net(&net);
+    XMEMSET(&broker, 0, sizeof(broker));
+    ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Init(&broker, &net));
+    broker.auth_user = "alice";
+    broker.auth_pass = "s3cr3tPW";
+    ASSERT_EQ(MQTT_CODE_SUCCESS, MqttBroker_Start(&broker));
+
+    reset_mock_state(connect, sizeof(connect));
+    run_broker_one_connect(&broker);
+
+    /* Auth accepted (0x00) and the client is retained. */
+    ASSERT_TRUE(g_out_len >= 4);
+    ASSERT_EQ(MQTT_CONNECT_ACK_CODE_ACCEPTED, g_out_buf[3]);
+    ASSERT_FALSE(g_client_closed);
+
+    bc = find_broker_client(&broker, "id");
+    ASSERT_TRUE(bc != NULL);
+
+    /* Password copy wiped: verify the length is cleared AND the buffer bytes
+     * are zeroed as independent checks - a regression that dropped only the
+     * buffer wipe while keeping password_len = 0 must still be caught. The
+     * dynamic store allocates password_len+1 bytes, so scanning the 8-byte
+     * plaintext stays within the allocation. */
+    ASSERT_EQ(0, (int)bc->password_len);
+    ASSERT_EQ(0, region_contains((const byte*)bc->password, 8, "s3cr3tPW", 8));
+    /* Both credential fields lived in the decoded CONNECT; neither plaintext
+     * may remain in the receive buffer. Scan only the received packet region
+     * (== the scrubbed rx_len); rx_buf is malloc'd without zeroing, so the
+     * bytes past the packet are uninitialized and must not be read. */
+    ASSERT_EQ(0, region_contains(bc->rx_buf, (int)sizeof(connect),
+        "s3cr3tPW", 8));
+    ASSERT_EQ(0, region_contains(bc->rx_buf, (int)sizeof(connect),
+        "alice", 5));
+
+    MqttBroker_Stop(&broker);
+    MqttBroker_Free(&broker);
+}
+#endif /* WOLFMQTT_BROKER_AUTH */
+
 #ifdef WOLFMQTT_V5
 /* Return the Reason Code byte of the first DISCONNECT packet in a captured
  * stream, or -1 if none carries a reason code. A v5 DISCONNECT with a
@@ -3265,6 +3349,9 @@ int main(int argc, char** argv)
     RUN_TEST(connect_auth_pass_only_start_rejected);
     RUN_TEST(connect_auth_partial_config_fails_closed);
     RUN_TEST(connect_auth_partial_pass_only_fails_closed);
+#ifndef WOLFMQTT_STATIC_MEMORY
+    RUN_TEST(connect_credentials_scrubbed_after_accept);
+#endif
 #endif
 #ifdef WOLFMQTT_V5
     RUN_TEST(connect_v5_emptyid_assigned_id_emitted);

From b7bae213e3a05c6679546f2025e785637d9ae2f7 Mon Sep 17 00:00:00 2001
From: Eric Blankenhorn <eric@wolfssl.com>
Date: Mon, 6 Jul 2026 13:57:26 -0500
Subject: [PATCH 4/5] Fix f-6656: scrub v5 CONNACK auth data from client rx_buf

---
 src/mqtt_client.c        | 22 ++++++++++++++++
 tests/test_mqtt_client.c | 56 ++++++++++++++++++++++++++++++++++++++++
 2 files changed, 78 insertions(+)

diff --git a/src/mqtt_client.c b/src/mqtt_client.c
index 1b8a91b49..174cd3c2e 100644
--- a/src/mqtt_client.c
+++ b/src/mqtt_client.c
@@ -1968,6 +1968,28 @@ int MqttClient_Connect(MqttClient *client, MqttConnect *mc_connect)
     }
 #endif
 
+#ifdef WOLFMQTT_V5
+    /* Scrub the decoded v5 CONNACK from rx_buf. Its properties (e.g. the
+     * MQTT_PROP_AUTH_DATA SASL server-final blob used by enhanced
+     * authentication) decode as pointers into rx_buf and would otherwise
+     * linger until the next read overwrites them - for an idle or QoS-0-only
+     * client, potentially the process lifetime. MqttClient_WaitType already
+     * delivered and freed the property list above, so the bytes are consumed
+     * and no live pointer into rx_buf remains. Same hardening as
+     * MqttClient_Auth; v3.1.1 CONNACK carries no properties so gate on v5. */
+    if (mc_connect->protocol_level > MQTT_CONNECT_PROTOCOL_LEVEL_4) {
+    #ifdef WOLFMQTT_MULTITHREAD
+        /* Hold lockRecv so the scrub cannot race a concurrent rx_buf read. */
+        if (wm_SemLock(&client->lockRecv) == 0) {
+            CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len);
+            wm_SemUnlock(&client->lockRecv);
+        }
+    #else
+        CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len);
+    #endif
+    }
+#endif
+
     /* reset state */
     mc_connect->stat.write = MQTT_MSG_BEGIN;
 
diff --git a/tests/test_mqtt_client.c b/tests/test_mqtt_client.c
index f98029c8c..4131500c2 100644
--- a/tests/test_mqtt_client.c
+++ b/tests/test_mqtt_client.c
@@ -590,6 +590,61 @@ TEST(connect_accepted_connack_latches_v5_props)
     ASSERT_EQ(1024, test_client.packet_sz_max);
 }
 
+/* f-6656: an accepted v5 CONNACK can carry MQTT_PROP_AUTH_DATA - the final
+ * SASL server signature for enhanced authentication - which decodes as a
+ * pointer into rx_buf (no copy). After MqttClient_Connect returns, that
+ * plaintext must not linger in rx_buf, where for an idle or QoS-0-only client
+ * it could persist for the process lifetime. Feed a CONNACK with a distinctive
+ * AUTH_DATA blob and assert it is gone from rx_buf afterward. Mirrors the
+ * MqttClient_Auth scrub (issue 4059); deleting the CLIENT_FORCE_ZERO in
+ * MqttClient_Connect (or turning it into a length-0 no-op) fails this test. */
+TEST(connect_v5_scrubs_connack_auth_data_from_rx_buf)
+{
+    int rc;
+    int i;
+    MqttConnect connect;
+    /* CONNACK v5: type=0x20, remain=0x14, flags=0x00, return_code=0x00,
+     * prop_len=0x11, then AUTH_DATA (0x16) binary length 14 =
+     * "SASLfinalPROOF". */
+    static const byte connack[] = {
+        0x20, 0x14, 0x00, 0x00, 0x11,
+        0x16, 0x00, 0x0E,
+        'S', 'A', 'S', 'L', 'f', 'i', 'n', 'a', 'l', 'P', 'R', 'O', 'O', 'F'
+    };
+    static const char blob[] = "SASLfinalPROOF";
+    const int blob_len = (int)sizeof(blob) - 1;
+
+    rc = test_init_client();
+    ASSERT_EQ(MQTT_CODE_SUCCESS, rc);
+    test_client.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5;
+
+    test_net.write = mock_net_write_accept;
+    test_net.read = mock_net_read_canned;
+    XMEMCPY(g_canned_buf, connack, sizeof(connack));
+    g_canned_len = (int)sizeof(connack);
+    g_canned_pos = 0;
+
+    XMEMSET(&connect, 0, sizeof(connect));
+    connect.keep_alive_sec = 60;
+    connect.clean_session = 1;
+    connect.client_id = "test_client";
+
+    rc = MQTT_CODE_CONTINUE;
+    for (i = 0; i < 10 && rc == MQTT_CODE_CONTINUE; i++) {
+        rc = MqttClient_Connect(&test_client, &connect);
+    }
+
+    /* Success + accepted proves the AUTH_DATA CONNACK was decoded (and thus
+     * was resident in rx_buf). */
+    ASSERT_EQ(MQTT_CODE_SUCCESS, rc);
+    ASSERT_EQ(MQTT_CONNECT_ACK_CODE_ACCEPTED, connect.ack.return_code);
+    /* The AUTH_DATA plaintext must not remain anywhere in rx_buf. rx_buf was
+     * zero-initialized at setup, so scanning the full length reads only
+     * initialized bytes. */
+    ASSERT_FALSE(buf_contains(test_client.rx_buf, test_client.rx_buf_len,
+                             blob, blob_len));
+}
+
 /* MQTT v5: a refused CONNACK (non-zero return code) must NOT mutate long-lived
  * client state even when it carries server properties, otherwise a rejected or
  * malicious broker could shrink the client's packet-size cap or lower its QoS
@@ -1847,6 +1902,7 @@ void run_mqtt_client_tests(void)
     RUN_TEST(connect_refused_connack_returns_connect_refused);
 #if defined(WOLFMQTT_V5)
     RUN_TEST(connect_accepted_connack_latches_v5_props);
+    RUN_TEST(connect_v5_scrubs_connack_auth_data_from_rx_buf);
     RUN_TEST(connect_refused_connack_preserves_v5_defaults);
     RUN_TEST(connect_accepted_connack_clamps_illegal_max_qos);
 #endif

From a336e5400a558c3c43114c8908b8a38bdb77f2a1 Mon Sep 17 00:00:00 2001
From: Eric Blankenhorn <eric@wolfssl.com>
Date: Fri, 10 Jul 2026 17:22:52 -0500
Subject: [PATCH 5/5] Fixes from review

---
 src/mqtt_client.c           | 14 ++++++--
 src/mqtt_packet.c           | 10 +++++-
 tests/test_broker_connect.c | 33 +++++-------------
 tests/test_mqtt_client.c    | 12 +++----
 tests/test_mqtt_packet.c    | 67 +++++++++++++++++++++++++++++++------
 wolfmqtt/mqtt_packet.h      |  6 ++--
 6 files changed, 93 insertions(+), 49 deletions(-)

diff --git a/src/mqtt_client.c b/src/mqtt_client.c
index 174cd3c2e..5ae009394 100644
--- a/src/mqtt_client.c
+++ b/src/mqtt_client.c
@@ -1979,11 +1979,16 @@ int MqttClient_Connect(MqttClient *client, MqttConnect *mc_connect)
      * MqttClient_Auth; v3.1.1 CONNACK carries no properties so gate on v5. */
     if (mc_connect->protocol_level > MQTT_CONNECT_PROTOCOL_LEVEL_4) {
     #ifdef WOLFMQTT_MULTITHREAD
-        /* Hold lockRecv so the scrub cannot race a concurrent rx_buf read. */
+        /* Hold lockRecv so the scrub cannot race a concurrent rx_buf read. If
+         * the lock cannot be taken, still scrub: leaving the AUTH_DATA
+         * plaintext behind is worse than an unsynchronized wipe. */
         if (wm_SemLock(&client->lockRecv) == 0) {
             CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len);
             wm_SemUnlock(&client->lockRecv);
         }
+        else {
+            CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len);
+        }
     #else
         CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len);
     #endif
@@ -3026,11 +3031,16 @@ int MqttClient_Auth(MqttClient *client, MqttAuth* auth)
      * already delivered and freed auth->props, so the bytes are consumed
      * before this scrub and the caller has no live pointer into rx_buf. */
 #ifdef WOLFMQTT_MULTITHREAD
-    /* Hold lockRecv so the scrub cannot race a concurrent read into rx_buf. */
+    /* Hold lockRecv so the scrub cannot race a concurrent read into rx_buf. If
+     * the lock cannot be taken, still scrub: leaving the AUTH_DATA plaintext
+     * behind is worse than an unsynchronized wipe. */
     if (wm_SemLock(&client->lockRecv) == 0) {
         CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len);
         wm_SemUnlock(&client->lockRecv);
     }
+    else {
+        CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len);
+    }
 #else
     CLIENT_FORCE_ZERO(client->rx_buf, client->rx_buf_len);
 #endif
diff --git a/src/mqtt_packet.c b/src/mqtt_packet.c
index 93f717552..fc51f0991 100644
--- a/src/mqtt_packet.c
+++ b/src/mqtt_packet.c
@@ -2576,6 +2576,13 @@ int MqttEncode_Subscribe(byte *tx_buf, int tx_buf_len,
          * Handling (bits 2-5). These bits are reserved (MUST be 0) in
          * v3.1.1, so only fold them in for a v5 SUBSCRIBE. */
         if (subscribe->protocol_level >= MQTT_CONNECT_PROTOCOL_LEVEL_5) {
+            /* Reject options the decoder would treat as malformed: bits
+             * outside 2-5 must be 0 and Retain Handling (bits 4-5) = 3 is
+             * reserved, so the encoder cannot emit an undecodable SUBSCRIBE. */
+            if ((topic->sub_options & ~MQTT_SUBSCRIBE_OPTIONS_MASK) != 0 ||
+                ((topic->sub_options >> 4) & 0x03) == 0x03) {
+                return MQTT_TRACE_ERROR(MQTT_CODE_ERROR_MALFORMED_DATA);
+            }
             options |= (byte)(topic->sub_options & MQTT_SUBSCRIBE_OPTIONS_MASK);
         }
     #endif
@@ -2727,7 +2734,8 @@ int MqttDecode_Subscribe(byte *rx_buf, int rx_buf_len, MqttSubscribe *subscribe)
             /* Preserve v5 No Local / Retain As Published / Retain Handling
              * (bits 2-5) so the full options byte round-trips. Reserved and
              * QoS bits were validated above; for a valid v3.1.1 packet these
-             * bits are 0. */
+             * bits are 0. The broker stores these but does not yet enforce
+             * them on delivery. */
             topic->sub_options = (byte)(options & MQTT_SUBSCRIBE_OPTIONS_MASK);
         #endif
             subscribe->topic_count++;
diff --git a/tests/test_broker_connect.c b/tests/test_broker_connect.c
index e7ae899c5..280f83fa9 100644
--- a/tests/test_broker_connect.c
+++ b/tests/test_broker_connect.c
@@ -590,22 +590,11 @@ TEST(connect_v311_binary_password_exact_match_accepted)
     MqttBroker_Free(&broker);
 }
 
-/* Mutation guard for the length-fold backstop in BrokerBufCompare (the
- * final "result |= (len_a ^ len_b)"). The per-byte constant-time loop
- * clamps an out-of-range index to position 0, so it is blind to a length
- * mismatch whenever the shorter input's bytes repeat through the longer
- * one: comparing configured "aaaaa" against a supplied "a" sees only
- * 'a' ^ 'a' == 0 at every clamped position, and only the length fold
- * rejects it. Every other negative auth test differs in actual byte
- * content within range, so the byte loop alone rejects those and none of
- * them exercise this line; deleting the fold would let a configured user
- * "aaaaa" be authenticated by the username "a".
- *
- * The username here is the repeating-byte length case while the password
- * matches auth_pass exactly, so the password compare is neutral and the
- * username length fold alone decides the outcome. Auth must fail (CONNACK
- * 0x04 Bad user/pass, connection closed). With the fold deleted this would
- * accept (return code 0x00) - the mutation-detecting assertion. */
+/* Guard the length-fold backstop in BrokerBufCompare. The constant-time
+ * byte loop clamps an out-of-range index to position 0, so it cannot see a
+ * length mismatch when the shorter input's bytes repeat through the longer
+ * one. Username "a" against configured "aaaaa" must be refused by the length
+ * fold alone; deleting the fold would authenticate it. */
 TEST(connect_auth_username_length_fold_repeating_byte_refused)
 {
     MqttBroker broker;
@@ -1005,14 +994,10 @@ static int region_contains(const byte* hay, int hlen,
     return 0;
 }
 
-/* f-3394: after an accepted CONNECT the plaintext credentials must not
- * linger for the connection's lifetime. The decoder leaves mc.username /
- * mc.password as in-place pointers into bc->rx_buf, and the broker copies the
- * password into bc->password only for the one-time auth compare. Neither is
- * read again (the broker has no re-auth path), so BrokerHandle_Connect scrubs
- * bc->rx_buf and bc->password before returning on the accepted path. Verify
- * the password plaintext is gone from rx_buf and bc->password_len is cleared
- * after a successful auth. Dynamic-memory only (uses find_broker_client). */
+/* After an accepted CONNECT the plaintext credentials must not linger for
+ * the connection lifetime. BrokerHandle_Connect scrubs bc->rx_buf and
+ * bc->password on the accepted path; verify the password is gone from rx_buf
+ * and bc->password_len is cleared. Dynamic-memory only. */
 TEST(connect_credentials_scrubbed_after_accept)
 {
     MqttBroker broker;
diff --git a/tests/test_mqtt_client.c b/tests/test_mqtt_client.c
index 4131500c2..406faaae7 100644
--- a/tests/test_mqtt_client.c
+++ b/tests/test_mqtt_client.c
@@ -590,14 +590,10 @@ TEST(connect_accepted_connack_latches_v5_props)
     ASSERT_EQ(1024, test_client.packet_sz_max);
 }
 
-/* f-6656: an accepted v5 CONNACK can carry MQTT_PROP_AUTH_DATA - the final
- * SASL server signature for enhanced authentication - which decodes as a
- * pointer into rx_buf (no copy). After MqttClient_Connect returns, that
- * plaintext must not linger in rx_buf, where for an idle or QoS-0-only client
- * it could persist for the process lifetime. Feed a CONNACK with a distinctive
- * AUTH_DATA blob and assert it is gone from rx_buf afterward. Mirrors the
- * MqttClient_Auth scrub (issue 4059); deleting the CLIENT_FORCE_ZERO in
- * MqttClient_Connect (or turning it into a length-0 no-op) fails this test. */
+/* An accepted v5 CONNACK can carry MQTT_PROP_AUTH_DATA, which decodes as a
+ * pointer into rx_buf with no copy. After MqttClient_Connect returns that
+ * plaintext must not linger in rx_buf. Feed a CONNACK with a distinctive
+ * AUTH_DATA blob and assert it is scrubbed. */
 TEST(connect_v5_scrubs_connack_auth_data_from_rx_buf)
 {
     int rc;
diff --git a/tests/test_mqtt_packet.c b/tests/test_mqtt_packet.c
index b8c0c60b6..3eea48884 100644
--- a/tests/test_mqtt_packet.c
+++ b/tests/test_mqtt_packet.c
@@ -1850,12 +1850,9 @@ TEST(encode_subscribe_options_byte_qos2)
 }
 
 #ifdef WOLFMQTT_V5
-/* f-2359: the v5 option bits (No Local / Retain As Published / Retain
- * Handling) MUST NOT appear on a v3.1.1 SUBSCRIBE, where bits 2-7 of the
- * options byte are reserved and MUST be 0. With protocol_level < 5 the
- * encoder must ignore sub_options and emit a QoS-only options byte. Locks
- * the protocol_level guard so a refactor that always folded sub_options
- * (emitting reserved bits on a 3.1.1 wire) can't pass CI. */
+/* The v5 option bits (bits 2-5) are reserved on a v3.1.1 SUBSCRIBE and MUST
+ * be 0. With protocol_level < 5 the encoder must ignore sub_options and emit
+ * a QoS-only options byte. */
 TEST(encode_subscribe_v311_ignores_sub_options)
 {
     byte tx_buf[256];
@@ -1880,6 +1877,53 @@ TEST(encode_subscribe_v311_ignores_sub_options)
     /* Options byte is the last byte: QoS 1 only, no v5 option bits. */
     ASSERT_EQ(0x01, tx_buf[rc - 1]);
 }
+
+/* Retain Handling = 3 is reserved [MQTT v5 3.8.3.1] and the decoder rejects
+ * it, so the encoder must refuse it too rather than emit a packet its own
+ * decoder would reject. */
+TEST(encode_subscribe_v5_retain_handling_3_rejected)
+{
+    byte tx_buf[256];
+    MqttSubscribe sub;
+    MqttTopic topic;
+    int rc;
+
+    XMEMSET(&sub, 0, sizeof(sub));
+    XMEMSET(&topic, 0, sizeof(topic));
+    topic.topic_filter = "a";
+    topic.qos = MQTT_QOS_1;
+    /* Bits 4-5 both set is Retain Handling = 3 (reserved). */
+    topic.sub_options = MQTT_SUBSCRIBE_RETAIN_HANDLING_1 |
+                        MQTT_SUBSCRIBE_RETAIN_HANDLING_2;
+    sub.topics = &topic;
+    sub.topic_count = 1;
+    sub.packet_id = 1;
+    sub.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5;
+    rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub);
+    ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc);
+}
+
+/* Bits 6-7 are reserved [MQTT v5 3.8.3.1]; sub_options should only hold bits
+ * 2-5, so any bit outside that mask must be refused by the encoder. */
+TEST(encode_subscribe_v5_reserved_sub_options_bit_rejected)
+{
+    byte tx_buf[256];
+    MqttSubscribe sub;
+    MqttTopic topic;
+    int rc;
+
+    XMEMSET(&sub, 0, sizeof(sub));
+    XMEMSET(&topic, 0, sizeof(topic));
+    topic.topic_filter = "a";
+    topic.qos = MQTT_QOS_1;
+    topic.sub_options = 0x40; /* reserved bit 6 */
+    sub.topics = &topic;
+    sub.topic_count = 1;
+    sub.packet_id = 1;
+    sub.protocol_level = MQTT_CONNECT_PROTOCOL_LEVEL_5;
+    rc = MqttEncode_Subscribe(tx_buf, (int)sizeof(tx_buf), &sub);
+    ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc);
+}
 #endif /* WOLFMQTT_V5 */
 
 /* f-2360: topic_filter with strlen > 65535 must be rejected with a negative
@@ -3519,11 +3563,10 @@ TEST(decode_subscribe_v5_options_reserved_bits_rejected)
     ASSERT_EQ(MQTT_CODE_ERROR_MALFORMED_DATA, rc);
 }
 
-/* f-2359: the v5 SUBSCRIBE options byte carries No Local (bit 2), Retain As
- * Published (bit 3) and Retain Handling (bits 4-5) alongside QoS (bits 0-1).
- * Encode then decode must preserve the full options byte via
- * MqttTopic.sub_options, not just QoS. Exhaustively covers every valid
- * combination: QoS 0-2 x No Local x RAP x Retain Handling 0-2 (36 cases). */
+/* The v5 SUBSCRIBE options byte carries No Local, Retain As Published and
+ * Retain Handling (bits 2-5) alongside QoS (bits 0-1). Encode then decode
+ * must preserve the full options byte via MqttTopic.sub_options. Covers
+ * every valid combination: QoS 0-2 x NL x RAP x Retain Handling 0-2. */
 TEST(subscribe_v5_options_roundtrip_all_combos)
 {
     static const byte rh_vals[] = {
@@ -5069,6 +5112,8 @@ void run_mqtt_packet_tests(void)
     RUN_TEST(encode_subscribe_options_byte_qos2);
 #ifdef WOLFMQTT_V5
     RUN_TEST(encode_subscribe_v311_ignores_sub_options);
+    RUN_TEST(encode_subscribe_v5_retain_handling_3_rejected);
+    RUN_TEST(encode_subscribe_v5_reserved_sub_options_bit_rejected);
 #endif
     RUN_TEST(encode_subscribe_topic_filter_oversized_rejected);
     RUN_TEST(encode_subscribe_topic_filter_oversized_second_rejected);
diff --git a/wolfmqtt/mqtt_packet.h b/wolfmqtt/mqtt_packet.h
index 4a55130f7..d10ffd7d4 100644
--- a/wolfmqtt/mqtt_packet.h
+++ b/wolfmqtt/mqtt_packet.h
@@ -212,9 +212,9 @@ typedef enum _MqttQoS {
 /* MQTT v5 SUBSCRIBE options byte (section 3.8.3.1), stored in
  * MqttTopic.sub_options in their on-the-wire bit positions. QoS (bits 0-1)
  * is carried separately in MqttTopic.qos; bits 6-7 are reserved (MUST be 0).
- * The encoder trusts the caller and emits sub_options verbatim (masked to
- * bits 2-5), so supply only spec-valid combinations - in particular Retain
- * Handling must be 0-2 (the value 3 is reserved and a broker will reject it). */
+ * The v5 encoder rejects sub_options with any bit outside 2-5 set, or with
+ * Retain Handling = 3 (reserved), returning MQTT_CODE_ERROR_MALFORMED_DATA;
+ * supply only spec-valid combinations. */
 #define MQTT_SUBSCRIBE_NO_LOCAL             0x04 /* bit 2 */
 #define MQTT_SUBSCRIBE_RETAIN_AS_PUBLISHED  0x08 /* bit 3 */
 #define MQTT_SUBSCRIBE_RETAIN_HANDLING_0    0x00 /* bits 4-5: send at subscribe */