Added Added more explicit error checking on WritePropertyMultiple decoding (untested).

bacnet-stack/demo/handler/h_wpm.c

@@ -36,6 +36,7 @@
 #include "npdu.h"
 #include "abort.h"
 #include "wp.h"
+#include "reject.h"
 #include "wpm.h"
 /* device object has the handling for all objects */
 #include "device.h"
@@ -61,9 +62,6 @@
  * @param service_data [in] The BACNET_CONFIRMED_SERVICE_DATA information
  *                          decoded from the APDU header of this message.
  */
-
-
-
 void handler_write_property_multiple(
     uint8_t * service_request,
     uint16_t service_len,
@@ -81,13 +79,9 @@ void handler_write_property_multiple(
     BACNET_ADDRESS my_address;
     int bytes_sent = 0;
 
-
-
     if (service_data->segmented_message) {
-        len =
+        wp_data.error_code = ERROR_CODE_ABORT_SEGMENTATION_NOT_SUPPORTED;
-            abort_encode_apdu(&Handler_Transmit_Buffer[npdu_len],
+        len = BACNET_STATUS_ABORT;
-            service_data->invoke_id, ABORT_REASON_SEGMENTATION_NOT_SUPPORTED,
-            true);
 #if PRINT_ENABLED
         fprintf(stderr, "WPM: Segmented message.  Sending Abort!\n");
 #endif
@@ -127,16 +121,14 @@ void handler_write_property_multiple(
 #endif
                         if (Device_Write_Property(&wp_data) == false) {
                             error = true;
-                            break;      /* do while (decoding List of Properties) */
+                            goto WPM_ABORT;
                         }
                     } else {
 #if PRINT_ENABLED
                         fprintf(stderr, "WPM: Bad Encoding!\n");
 #endif
-                        wp_data.error_class = ERROR_CLASS_PROPERTY;
-                        wp_data.error_code = ERROR_CODE_OTHER;
                         error = true;
-                        break;  /* do while (decoding List of Properties) */
+                        goto WPM_ABORT;
                     }
 
                     /* Closing tag 1 - List of Properties */
@@ -150,49 +142,63 @@ void handler_write_property_multiple(
                 }
                 while (tag_number != 1);        /* end decoding List of Properties for "that" object */
 
-                if (error)
+                if (error) {
-                    break;      /*do while (decode service request) */
+                    goto WPM_ABORT;
+                }
             }
         } else {
 #if PRINT_ENABLED
             fprintf(stderr, "WPM: Bad Encoding!\n");
 #endif
-            wp_data.error_class = ERROR_CLASS_OBJECT;
-            wp_data.error_code = ERROR_CODE_OTHER;
             error = true;
-            break;      /*do while (decode service request) */
+            goto WPM_ABORT;
         }
-    }
+    } while (decode_len < service_len);
-    while (decode_len < service_len);
-
 
+WPM_ABORT:
     /* encode the NPDU portion of the packet */
     datalink_get_my_address(&my_address);
     npdu_encode_npdu_data(&npdu_data, false, MESSAGE_PRIORITY_NORMAL);
     npdu_len =
         npdu_encode_pdu(&Handler_Transmit_Buffer[0], src, &my_address,
         &npdu_data);
-
     apdu_len = 0;
-
+    /* handle any errors */
-    if (error == false) {
+    if (error) {
+        if (len == BACNET_STATUS_ABORT) {
+            apdu_len =
+                abort_encode_apdu(&Handler_Transmit_Buffer[npdu_len],
+                service_data->invoke_id,
+                abort_convert_error_code(wp_data.error_code), true);
+#if PRINT_ENABLED
+            fprintf(stderr, "WPM: Sending Abort!\n");
+#endif
+        } else if (len == BACNET_STATUS_ERROR) {
+            apdu_len =
+                bacerror_encode_apdu(&Handler_Transmit_Buffer[npdu_len],
+                service_data->invoke_id, SERVICE_CONFIRMED_WRITE_PROP_MULTIPLE,
+                wp_data.error_class, wp_data.error_code);
+#if PRINT_ENABLED
+            fprintf(stderr, "WPM: Sending Error!\n");
+#endif
+        } else if (len == BACNET_STATUS_REJECT) {
+            apdu_len =
+                reject_encode_apdu(&Handler_Transmit_Buffer[npdu_len],
+                service_data->invoke_id,
+                reject_convert_error_code(wp_data.error_code));
+#if PRINT_ENABLED
+            fprintf(stderr, "WPM: Sending Reject!\n");
+#endif
+        }
+    } else {
         apdu_len =
             wpm_ack_encode_apdu_init(&Handler_Transmit_Buffer[npdu_len],
             service_data->invoke_id);
 #if PRINT_ENABLED
-        fprintf(stderr, "WPM: Sending Simple Ack!\n");
+        fprintf(stderr, "WPM: Sending Ack!\n");
-#endif
-    } else {
-        apdu_len =
-            wpm_error_ack_encode_apdu(&Handler_Transmit_Buffer[npdu_len],
-            service_data->invoke_id, &wp_data);
-#if PRINT_ENABLED
-        fprintf(stderr, "WPM: Sending Error!\n");
 #endif
     }
 
-  WPM_ABORT:
-
     pdu_len = npdu_len + apdu_len;
     bytes_sent =
         datalink_send_pdu(src, &npdu_data, &Handler_Transmit_Buffer[0],

bacnet-stack/src/wpm.c

@@ -33,11 +33,25 @@
 
 /** @file wpm.c  Encode/Decode BACnet Write Property Multiple APDUs  */
 
-/* decode service */
+/** Decoding for WritePropertyMultiple service, object ID.
+ * @ingroup DSWPM
+ * This handler will be invoked by write_property_multiple handler
+ * if it has been enabled by a call to apdu_set_confirmed_handler().
+ * This function decodes only the first tagged entity, which is
+ * an object identifier.  This function will return an error if:
+ *   - the tag is not the right value
+ *   - the number of bytes is not enough to decode for this entity
+ *   - the subsequent tag number is incorrect
+ *
+ * @param apdu [in] The contents of the APDU buffer.
+ * @param apdu_len [in] The length of the APDU buffer.
+ * @param data [out] The BACNET_WRITE_PROPERTY_DATA structure
+ *    which will contain the reponse values or error.
+ */
 int wpm_decode_object_id(
     uint8_t * apdu,
     uint16_t apdu_len,
-    BACNET_WRITE_PROPERTY_DATA * data)
+    BACNET_WRITE_PROPERTY_DATA * wp_data)
 {
     uint8_t tag_number = 0;
     uint32_t len_value = 0;
@@ -45,19 +59,36 @@ int wpm_decode_object_id(
     uint16_t object_type = 0;
     uint16_t len = 0;
 
-    if ((apdu) && (apdu_len)) {
+    if (apdu && (apdu_len > 5) && wp_data) {
         /* Context tag 0 - Object ID */
         len +=
             decode_tag_number_and_value(&apdu[len], &tag_number, &len_value);
-        if (tag_number == 0) {
+        if ((tag_number == 0) && (apdu_len > len)) {
-            len +=
+            apdu_len -= len;
-                decode_object_id(&apdu[len], &object_type, &object_instance);
+            if (apdu_len >= 4) {
-            data->object_type = object_type;
+                len +=
-            data->object_instance = object_instance;
+                    decode_object_id(&apdu[len], &object_type, &object_instance);
-        } else
+                wp_data->object_type = object_type;
-            return -1;
+                wp_data->object_instance = object_instance;
-    } else
+                apdu_len -= len;
-        return -1;
+            } else {
+                wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER;
+                return BACNET_STATUS_REJECT;
+            }
+        } else {
+            wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
+            return BACNET_STATUS_REJECT;
+        }
+        /* just test for the next tag - no need to decode it here */
+        /* Context tag 1: sequence of BACnetPropertyValue */
+        if (apdu_len && !decode_is_opening_tag_number(&apdu[len], 1)) {
+            wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
+            return BACNET_STATUS_REJECT;
+        }
+    } else {
+        wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER;
+        return BACNET_STATUS_REJECT;
+    }
 
     return (int)len;
 }
@@ -78,15 +109,16 @@ int wpm_decode_object_property(
         wp_data->array_index = BACNET_ARRAY_ALL;
         wp_data->priority = BACNET_MAX_PRIORITY;
         wp_data->application_data_len = 0;
-
         /* tag 0 - Property Identifier */
         len +=
             decode_tag_number_and_value(&apdu[len], &tag_number, &len_value);
         if (tag_number == 0) {
             len += decode_enumerated(&apdu[len], len_value, &ulVal);
             wp_data->object_property = ulVal;
-        } else
+        } else {
-            return -1;
+            wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
+            return BACNET_STATUS_REJECT;
+        }
 
         /* tag 1 - Property Array Index - optional */
         len +=
@@ -108,18 +140,22 @@ int wpm_decode_object_property(
             len++;
 
             /* copy application data */
-            for (i = 0; i < wp_data->application_data_len; i++)
+            for (i = 0; i < wp_data->application_data_len; i++) {
                 wp_data->application_data[i] = apdu[len + i];
+            }
             len += wp_data->application_data_len;
-
             len +=
                 decode_tag_number_and_value(&apdu[len], &tag_number,
                 &len_value);
             /* closing tag 2 */
-            if ((tag_number != 2) && (decode_is_closing_tag(&apdu[len - 1])))
+            if ((tag_number != 2) && (decode_is_closing_tag(&apdu[len - 1]))) {
-                return -1;
+                wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
-        } else
+                return BACNET_STATUS_REJECT;
-            return -1;
+            }
+        } else {
+            wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
+            return BACNET_STATUS_REJECT;
+        }
 
         /* tag 3 - Priority - optional */
         len +=
@@ -129,8 +165,10 @@ int wpm_decode_object_property(
             wp_data->priority = ulVal;
         } else
             len--;
-    } else
+    } else {
-        return -1;
+        wp_data->error_code = ERROR_CODE_REJECT_MISSING_REQUIRED_PARAMETER;
+        return BACNET_STATUS_REJECT;
+    }
 
     return len;
 }