Skip to content

Conversation

@kaabia
Copy link
Contributor

@kaabia kaabia commented Nov 1, 2025

Fixes #443

Summary

This pull request implements the required MQTT v5 protocol validation for properties and adds several buffer safety checks in the property decoding logic.

Key Changes

  1. Protocol Compliance (Property Validation):

    • Implements the check in both MqttEncode_Props and MqttDecode_Props to ensure a property is only used in packet types defined by its packet_type_mask in gPropMatrix. This resolves the explicit TODO in both functions.
    • Introduces the new error code MQTT_CODE_ERROR_PROPERTY_MISMATCH for clear reporting of protocol violations.
  2. Buffer Safety:

    • Added explicit boundary checks in MqttDecode_Props before decoding the property identifier (VBI) and before decoding property string data. This prevents potential buffer overruns if a packet length field is malformed or manipulated.

@wolfSSL-Bot
Copy link

Can one of the admins verify this patch?

@embhorn
Copy link
Member

embhorn commented Nov 5, 2025

Hello @kaabia

Thanks for sharing this PR! Could you tell us a bit about your interest in the wolfMQTT project? We require all contributors to provide a signed agreement. You can send an email mentioning this PR to [email protected] to start this process.

Kind regards,
Eric - wolfSSL Support

@dgarske
Copy link
Contributor

dgarske commented Nov 5, 2025

Hello @kaabia

Thanks for sharing this PR! Could you tell us a bit about your interest in the wolfMQTT project? We require all contributors to provide a signed agreement. You can send an email mentioning this PR to [email protected] to start this process.

Kind regards, Eric - wolfSSL Support

Hey @embhorn We got a signed contributor agreement from @kaabia , but I haven't see yet if its approved. See ZD 20757. He also submitted wolfSSL/wolfBoot#627

@kaabia
Copy link
Contributor Author

kaabia commented Nov 5, 2025

Thank you for your review.
I also want to mention here also that the test gate is currently failing due to expired certificates that need updating in the test suite:

[2025/11/01 15:40:16:6281] E: SSL error: certificate has expired (preverify_ok=0;err=10;depth=1)

see #441
Thank you!

@embhorn
Copy link
Member

embhorn commented Nov 6, 2025

Hi @kaabia

The fix for the expired certs is in. We are waiting on one more fix for the Espressif test. I'll let you know when to rebase and push.

@embhorn
Copy link
Member

embhorn commented Nov 6, 2025

Hi @kaabia

Please rebase and push to restart the tests with the test fixes in place.

@embhorn embhorn assigned kaabia and unassigned wolfSSL-Bot and embhorn Nov 6, 2025
@kaabia kaabia force-pushed the fix/mqtt5-prop-packet-validation branch from d8805da to 2971607 Compare November 6, 2025 21:08
@kaabia
Copy link
Contributor Author

kaabia commented Nov 6, 2025

Hi @kaabia

Please rebase and push to restart the tests with the test fixes in place.

Hi @embhorn,
Rebase done
Thank you for the notification.

Copy link
Member

@embhorn embhorn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for sharing these changes. I am requesting one minor change.

…safety

Implements necessary protocol validation in MqttEncode_Props and
MqttDecode_Props to ensure that properties are only used in their
allowed packet types, addressing the 'TODO: validate packet type'.

- Defines new error code: MQTT_CODE_ERROR_PROPERTY_MISMATCH.
- Adds critical buffer boundary checks in MqttDecode_Props before
  VBI and string decoding to prevent potential buffer overruns.

Signed-off-by: Badr Bacem KAABIA <[email protected]>
@kaabia kaabia force-pushed the fix/mqtt5-prop-packet-validation branch from 2971607 to b430f2c Compare November 7, 2025 18:20
@kaabia kaabia requested a review from embhorn November 7, 2025 18:22
@embhorn
Copy link
Member

embhorn commented Nov 7, 2025

Thanks for updating the PR. Looking forward to reviewing any more updates you want to share.

@embhorn embhorn merged commit 9a9d9dd into wolfSSL:master Nov 7, 2025
12 checks passed
@kaabia
Copy link
Contributor Author

kaabia commented Nov 7, 2025

Thanks for updating the PR. Looking forward to reviewing any more updates you want to share.

Thank you as well! I sincerely appreciate your review and continued support. I'll be sure to share any further updates as soon as they are ready. 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Missing MQTT v5 property protocol validation and decoding buffer safety checks

4 participants