Fix: Enforce MQTT v5 property-packet validation and improve decoding 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]>

Author: kaabia

src/mqtt_packet.c

@@ -377,13 +377,15 @@ int MqttEncode_Props(MqttPacketType packet, MqttProp* props, byte* buf)
 
     /* TODO: Check against max size. Sometimes all properties are not
              expected to be added */
-    /* TODO: Validate property type is allowed for packet type */
 
     /* loop through the list properties */
     while ((cur_prop != NULL) && (rc >= 0))
     {
-        /* TODO: validate packet type */
-        (void)packet;
+        /* Validate property type is allowed for this packet type */
+        if (!(gPropMatrix[cur_prop->type].packet_type_mask & (1 << packet))) {
+            rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PROPERTY_MISMATCH);
+            break;
+        }
 
         /* Encode the Identifier */
         tmp = MqttEncode_Vbi(buf, (word32)cur_prop->type);
@@ -505,11 +507,12 @@ int MqttDecode_Props(MqttPacketType packet, MqttProp** props, byte* pbuf,
             break;
         }
 
-        /* Decode the Identifier */
-        if (buf_len < (word32)(buf - pbuf)) {
+        /* Check boundary before VBI decoding */
+        if ((buf - pbuf) > (int)buf_len) {
             rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
             break;
         }
+        /* Decode the Identifier */
         rc = MqttDecode_Vbi(buf, (word32*)&cur_prop->type,
                 (word32)(buf_len - (buf - pbuf)));
         if (rc < 0) {
@@ -520,8 +523,11 @@ int MqttDecode_Props(MqttPacketType packet, MqttProp** props, byte* pbuf,
         total += tmp;
         prop_len -= tmp;
 
-        /* TODO: Validate property type is allowed for packet type */
-        (void)packet;
+        /* Validate property type is allowed for packet type */
+        if (!(gPropMatrix[cur_prop->type].packet_type_mask & (1 << packet))) {
+            rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PROPERTY_MISMATCH);
+            break;
+        }
 
         if (cur_prop->type >= sizeof(gPropMatrix) / sizeof(gPropMatrix[0])) {
             rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_PROPERTY);
@@ -561,6 +567,11 @@ int MqttDecode_Props(MqttPacketType packet, MqttProp** props, byte* pbuf,
             }
             case MQTT_DATA_TYPE_STRING:
             {
+                if ((buf - pbuf) > (int)buf_len) {
+                    /* Should already be caught earlier, but safe to recheck */
+                    rc = MQTT_TRACE_ERROR(MQTT_CODE_ERROR_OUT_OF_BUFFER);
+                    break;
+                }
                 tmp = MqttDecode_String(buf,
                         (const char**)&cur_prop->data_str.str,
                         &cur_prop->data_str.len,

wolfmqtt/mqtt_types.h

@@ -202,6 +202,7 @@ enum MqttPacketResponseCodes {
     MQTT_CODE_ERROR_CURL = -16, /* An error in libcurl that is not clearly
                                  * a network, memory, TLS, or system error. */
 #endif
+    MQTT_CODE_ERROR_PROPERTY_MISMATCH = -17,
 
     MQTT_CODE_CONTINUE = -101,
     MQTT_CODE_STDIN_WAKE = -102,