Broker, client, and MQTT v5 packet validation and reliability fixes#552
Open
aidangarske wants to merge 31 commits into
Open
Broker, client, and MQTT v5 packet validation and reliability fixes#552aidangarske wants to merge 31 commits into
aidangarske wants to merge 31 commits into
Conversation
… sub/unsub property cleanup
…shot sub iterator
…tize peer strings in broker log sinks
…gs in example log output
…erification, and log injection
…5 props, fix packet-size comparison
…ll re-entrancy, scoped wolfSSL cleanup, fan-out write reset
… PUBACK on topic alloc failure
…et credential staging buffer
…roker publish, enforce retained cap on persist restore
…cap at 1, count restore cap skips
…unsubscribe-reject test
… reap tombstones after delivery
embhorn
requested changes
Jun 12, 2026
embhorn
left a comment
Member
There was a problem hiding this comment.
Couple minor things to clean up
| MQTTCtx* mqttCtx = (MQTTCtx*)client->ctx; | ||
|
|
||
| /* Verify this message is for the firmware topic */ | ||
| /* Verify this message is for the firmware topic. Compare against the full |
Member
There was a problem hiding this comment.
Too wordy for the example. Revert this comment to the original.
| Certificate: | ||
| Data: | ||
| Version: 3 (0x2) | ||
| Version: 1 (0x0) |
Member
There was a problem hiding this comment.
Why did this change to V1 cert?
| while (((int)prop_len > 0) && (rc >= 0)) | ||
| { | ||
| /* Bound the number of properties a single message may carry so a peer | ||
| * cannot saturate the shared property pool (CWE-770). */ |
| MqttBroker_Free(&broker); | ||
| } | ||
|
|
||
| /* [CWE-863/CWE-639] An unauthenticated CONNECT must not mutate another |
| if (broker->retained_delivering > 0) { | ||
| /* A delivery loop is iterating this list (possibly re-entered | ||
| * via a WebSocket fan-out). Freeing now would invalidate that | ||
| * loop's saved next pointer (CWE-416); flag for deferred reap |
| ASSERT_NULL(pub.props); | ||
| } | ||
|
|
||
| /* [CWE-770] A single message may not carry more than the internal |
| ASSERT_TRUE(rc > 0); | ||
| } | ||
|
|
||
| /* [CWE-125] The encoder must clamp the copied payload to buffer_len so a |
| #endif /* WOLFMQTT_BROKER_RETAINED && !WOLFMQTT_STATIC_MEMORY */ | ||
|
|
||
| #ifndef WOLFMQTT_STATIC_MEMORY | ||
| /* [CWE-770] A single client cannot occupy more than BROKER_MAX_SUBS_PER_CLIENT |
| } | ||
|
|
||
| #if defined(WOLFMQTT_BROKER_RETAINED) && !defined(WOLFMQTT_STATIC_MEMORY) | ||
| /* [CWE-400] The dynamic retained-message list must be bounded. A client that |
| return n; | ||
| } | ||
|
|
||
| /* [issue 3129] A broker that rejects a subscription returns a SUBACK whose |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description