Fenrir fixes

[embhorn]

• Fix f-3627: test BrokerBufCompare length-fold auth backstop
• Fix f-2359: preserve v5 SUBSCRIBE options through encode/decode
• Fix f-3394: scrub CONNECT credentials from broker rx_buf after auth
• Fix f-6656: scrub v5 CONNACK auth data from client rx_buf

[Copilot]

Pull request overview

Updates the broker CONNECT authentication test suite to better cover a constant-time credential comparison edge case and to align comments with the broker’s actual CONNACK return code for bad credentials.

Changes:

• Corrects auth-failure CONNACK code references in existing tests/comments to 0x04 (Bad user name or password).
• Adds a new mutation-guard test that fails if the length-fold backstop in BrokerBufCompare is removed (repeating-byte, different-length username case).
• Registers the new test in the broker auth test runner.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #559

Scan targets checked: wolfmqtt-bugs, wolfmqtt-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

[aidangarske]

Skoll Multi-Scan ReviewOverall recommendation: COMMENT

Findings: 3 total — 3 posted, 0 skipped
2 finding(s) posted as inline comments (see file-level comments below)
1 finding(s) not tied to a diff line (full detail below)

Posted findings

• [Medium] [review+review-security+bugs] v5 SUBSCRIBE encoder folds uninitialized/unvalidated MqttTopic.sub_options into the options byte, emitting reserved or garbage option bits on the wire — src/mqtt_packet.c:2417-2436
• [Low] [review] Broker decodes v5 sub_options but never honors No Local / Retain As Published / Retain Handling — src/mqtt_packet.c:2575-2581

Findings not tied to a diff line

Credential/CONNACK scrub changes reviewed clean (informational)

File: src/mqtt_broker.c:4939-4957, src/mqtt_client.c:1970-1990
Function: BrokerHandle_Connect / MqttClient_Connect
Severity: Info
Category: Zeroization

The two new zeroization sites were audited for common failure modes and independently verified against the code as correct. Broker: BROKER_FORCE_ZERO(bc->rx_buf, rx_len) is in-bounds because rx_len is the MqttPacket_Read return, bounded by the rx buffer size; the dynamic-mode password scrub uses password_len + 1, matching the WOLFMQTT_MALLOC(src_len + 1) allocation and NULL-guarded; static-mode scrubs the full BROKER_MAX_PASSWORD_LEN array; password_len = 0 and the password scrub are correctly gated on WOLFMQTT_BROKER_AUTH. No double-free/UAF: the pointer is unchanged so the later BrokerClient_Free scrub+free path still works. Client: the whole-rx_buf scrub is safe because v5 CONNACK properties are consumed synchronously (MqttClient_WaitType delivers and frees the property list before the scrub) and no live pointer into rx_buf remains; ack.return_code is a scalar and survives; the multithread path takes lockRecv non-nested (no deadlock) and mirrors the existing MqttClient_Auth hardening. No defect found; recorded for traceability.

Recommendation: No action required.

Referenced code: src/mqtt_broker.c:4939-4957, src/mqtt_client.c:1970-1980 (11 lines)

---

Review generated by Skoll