Fenrir fixes in broker

[embhorn]

The following issues are fixed:

• Fix f805 broker check client connect status
• Fix f483 - lwt length check
• Fix f482 - wildcard matching
• Fix f797 - Check topic count for subAck
• f486 - subAck buf size check
• Fix f485 lwt length check
• Fix f811 - clear before free
• Fix f803 - lwt free payload
• Fix f804 - do not truncate retained message length
• Fix f488 - clear lwt topic and payload on free

[Copilot]

Pull request overview

This PR addresses multiple broker correctness and safety issues around CONNECT state tracking, LWT payload handling, retained message sizing, wildcard topic matching, and SUBACK bounds checking.

Changes:

• Add explicit client “connected” state enforcement so first packet must be CONNECT.
• Fix length-related issues: retained payload length widened to 32-bit, LWT payload length bounded, and SUBACK topic/count/buffer checks tightened.
• Improve cleanup behavior by clearing sensitive data before free and fixing missing frees.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.

File	Description
wolfmqtt/mqtt_broker.h	Adds connected state to BrokerClient and widens retained payload length to prevent truncation.
src/mqtt_broker.c	Enforces CONNECT-first rule, improves string storage/cleanup, fixes SUBACK bounds checks, adjusts retained/LWT length handling, and corrects memory frees.

---

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

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

This PR addresses multiple broker correctness and safety issues around client connection state, LWT handling, retained message sizing, wildcard topic matching, and SUBACK bounds/buffer checks.

Changes:

• Add connected state to broker clients and enforce CONNECT as the first packet.
• Harden LWT, retained message, and SUBACK handling (length/buffer checks, cleanup on free).
• Adjust wildcard matching and retained payload length type to avoid truncation/overrun.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
wolfmqtt/mqtt_broker.h	Adds client connected state; widens retained payload length to word32.
src/mqtt_broker.c	Adds sensitive string storage/wiping, fixes LWT/suback/retained handling, and enforces CONNECT-first processing.

Comments suppressed due to low confidence (1)

src/mqtt_broker.c:1

• In the non-static-memory build, BrokerStore_String allocates based on src_len without applying the maxlen cap that static-memory builds enforce. Since username/password lengths come from the incoming packet, this creates an avoidable memory-DoS risk and inconsistent behavior across build modes. Consider adding a max_len parameter (or applying a cap before calling) so dynamic builds also clamp to BROKER_MAX_*_LEN.

/* mqtt_broker.c

---

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

You can also share your feedback on Copilot code review. Take the survey.

[Copilot]

Pull request overview

This PR addresses multiple broker correctness and safety issues around CONNECT state tracking, LWT handling, wildcard matching, SUBACK bounds checks, and retained message length handling.

Changes:

• Track post-CONNECT handshake state (connected) and reject non-CONNECT first packets.
• Harden parsing/storage around LWT, username/password (including wiping on overwrite/free), and SUBACK bounds.
• Fix retained message payload length truncation and adjust related types.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

File	Description
wolfmqtt/mqtt_broker.h	Adds BrokerClient.connected state and widens retained payload length to word32 to prevent truncation.
src/mqtt_broker.c	Adds sensitive string storage/wiping, strengthens LWT/SUBACK checks, adjusts retained store APIs, and enforces CONNECT-first behavior.

Comments suppressed due to low confidence (1)

src/mqtt_broker.c:1

• bc->connected is set whenever BrokerHandle_Connect returns MQTT_CODE_SUCCESS, but BrokerHandle_Connect can still send a CONNACK with a refusal code (e.g., via ack.return_code = ...; goto send_connack;). In that case, marking the client as connected defeats the “first packet must be CONNECT” gate and can allow protocol-invalid traffic after a refused CONNECT. Set bc->connected only when the CONNECT handshake is accepted (CONNACK=Accepted), or have BrokerHandle_Connect return a distinct non-success code when the CONNACK return code is not Accepted (and ideally close/remove the client on refusal).

/* mqtt_broker.c

---

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

You can also share your feedback on Copilot code review. Take the survey.