Feature/zeroing rx buffer remain (#90)

* Added zeroing rx buffer remain

* Added zeroing rx buffer remain

* Added safety margin for the rx-buffer in the different ports.

* Added safety margin for the receive buffer.

* Added DoxyGen comments.

* Fixed checking return value when calculating distance between opening and closing tag on multiple properties.

ports/arduino_uno/bip.c

@@ -244,6 +244,7 @@ uint16_t bip_receive(BACNET_ADDRESS *src, /* source address */
     unsigned timeout)
 {
     int received_bytes = 0;
+    int max = 0;
     uint16_t pdu_len = 0; /* return value */
     uint8_t src_addr[] = { 0, 0, 0, 0 };
     uint16_t src_port = 0;
@@ -274,6 +275,18 @@ uint16_t bip_receive(BACNET_ADDRESS *src, /* source address */
     if (pdu[0] != BVLL_TYPE_BACNET_IP)
         return 0;
 
+    /* Erase up to 16 bytes after the received bytes as safety margin to
+     * ensure that the decoding functions will run into a 'safe field'
+     * of zero, if for any reason they would overrun, when parsing the
+     * message. */
+    max = (int)max_pdu - received_bytes;
+    if (max > 0) {
+        if (max > 16) {
+            max = 16;
+        }
+        memset(&pdu[received_bytes], 0, max);
+    }
+
     if (bvlc_for_non_bbmd(src_addr, &src_port, pdu, received_bytes) > 0) {
         /* Handled, usually with a NACK. */
 #if PRINT_ENABLED

ports/arduino_uno/main.c

@@ -70,7 +70,17 @@ void setup()
 #endif
 }
 
-static uint8_t PDUBuffer[MAX_MPDU];
+/** Static receive buffer, initialized with zeros by the C Library Startup Code. */
+
+static uint8_t PDUBuffer[MAX_MPDU + 16 /* Add a little safety margin to the buffer,
+                                        * so that in the rare case, the message
+                                        * would be filled up to MAX_MPDU and some
+                                        * decoding functions would overrun, these
+                                        * decoding functions will just end up in
+                                        * a safe field of static zeros. */];
+
+/** Main */
+
 int main(void)
 {
     uint16_t pdu_len = 0;
@@ -84,7 +94,7 @@ int main(void)
     for (;;) {
         /* other tasks */
         /* BACnet handling */
-        pdu_len = datalink_receive(&src, &PDUBuffer[0], sizeof(PDUBuffer), 0);
+        pdu_len = datalink_receive(&src, &PDUBuffer[0], MAX_MPDU, 0);
         if (pdu_len) {
             npdu_handler(&src, &PDUBuffer[0], pdu_len);
         }

ports/at91sam7s/main.c

@@ -162,7 +162,17 @@ static inline void bacnet_init(void)
         handler_device_communication_control);
 }
 
-static uint8_t Receive_PDU[MAX_MPDU]; /* PDU data */
+/** Static receive buffer, initialized with zeros by the C Library Startup Code. */
+
+static uint8_t Receive_PDU[MAX_MPDU + 16 /* Add a little safety margin to the buffer,
+                                          * so that in the rare case, the message
+                                          * would be filled up to MAX_MPDU and some
+                                          * decoding functions would overrun, these
+                                          * decoding functions will just end up in
+                                          * a safe field of static zeros. */];
+
+/** Main */
+
 int main(void)
 {
     unsigned long IdleCount = 0; /* idle loop blink counter */
@@ -240,7 +250,7 @@ int main(void)
         IdleCount++;
         /* BACnet handling */
         pdu_len =
-            datalink_receive(&src, &Receive_PDU[0], sizeof(Receive_PDU), 0);
+            datalink_receive(&src, &Receive_PDU[0], MAX_MPDU, 0);
         if (pdu_len) {
             pPIO->PIO_CODR = LED3;
             npdu_handler(&src, &Receive_PDU[0], pdu_len);

ports/atmega168/main.c

@@ -135,7 +135,18 @@ static void input_switch_read(void)
     }
 }
 
-static uint8_t PDUBuffer[MAX_MPDU];
+
+/** Static receive buffer, initialized with zeros by the C Library Startup Code. */
+
+static uint8_t PDUBuffer[MAX_MPDU + 16 /* Add a little safety margin to the buffer,
+                                        * so that in the rare case, the message
+                                        * would be filled up to MAX_MPDU and some
+                                        * decoding functions would overrun, these
+                                        * decoding functions will just end up in
+                                        * a safe field of static zeros. */];
+
+/** Main */
+
 int main(void)
 {
     uint16_t pdu_len = 0;
@@ -153,7 +164,7 @@ int main(void)
         task_milliseconds();
         /* other tasks */
         /* BACnet handling */
-        pdu_len = datalink_receive(&src, &PDUBuffer[0], sizeof(PDUBuffer), 0);
+        pdu_len = datalink_receive(&src, &PDUBuffer[0], MAX_MPDU, 0);
         if (pdu_len) {
             LED_NPDU_ON();
             npdu_handler(&src, &PDUBuffer[0], pdu_len);

ports/bdk-atxx4-mstp/bacnet.c

@@ -141,7 +141,17 @@ void bacnet_init(void)
     Send_I_Am(&Handler_Transmit_Buffer[0]);
 }
 
-static uint8_t PDUBuffer[MAX_MPDU];
+/** Static receive buffer, initialized with zeros by the C Library Startup Code. */
+
+static uint8_t PDUBuffer[MAX_MPDU + 16 /* Add a little safety margin to the buffer,
+                                        * so that in the rare case, the message
+                                        * would be filled up to MAX_MPDU and some
+                                        * decoding functions would overrun, these
+                                        * decoding functions will just end up in
+                                        * a safe field of static zeros. */];
+
+/** BACnet task doing receive and transmit. */
+
 void bacnet_task(void)
 {
     uint8_t mstp_mac_address;
@@ -208,7 +218,7 @@ void bacnet_task(void)
         dcc_timer_seconds(DCC_CYCLE_SECONDS);
     }
     /* handle the messaging */
-    pdu_len = datalink_receive(&src, &PDUBuffer[0], sizeof(PDUBuffer), 0);
+    pdu_len = datalink_receive(&src, &PDUBuffer[0], MAX_MPDU, 0);
     if (pdu_len) {
         npdu_handler(&src, &PDUBuffer[0], pdu_len);
     }

ports/bsd/bip-init.c

@@ -319,7 +319,7 @@ uint16_t bip_receive(
     max = BIP_Socket;
     /* see if there is a packet for us */
     if (select(max + 1, &read_fds, NULL, NULL, &select_timeout) > 0) {
-        received_bytes = recvfrom(BIP_Socket, (char *)&npdu[0], max_npdu, 0,
+        received_bytes = recvfrom(max, (char *)&npdu[0], max_npdu, 0,
             (struct sockaddr *)&sin, &sin_len);
     } else {
         return 0;
@@ -336,6 +336,17 @@ uint16_t bip_receive(
     if (npdu[0] != BVLL_TYPE_BACNET_IP) {
         return 0;
     }
+    /* Erase up to 16 bytes after the received bytes as safety margin to
+     * ensure that the decoding functions will run into a 'safe field'
+     * of zero, if for any reason they would overrun, when parsing the
+     * message. */
+    max = (int)max_npdu - received_bytes;
+    if (max > 0) {
+        if (max > 16) {
+            max = 16;
+        }
+        memset(&npdu[received_bytes], 0, max);
+    }
     /* Data link layer addressing between B/IPv4 nodes consists of a 32-bit
        IPv4 address followed by a two-octet UDP port number (both of which
        shall be transmitted with the most significant octet first). This

ports/bsd/main.c

@@ -51,8 +51,14 @@
 
 bool Who_Is_Request = true;
 
-/* buffers used for receiving */
+/** Static receive buffer, initialized with zeros by the C Library Startup Code. */
-static uint8_t Rx_Buf[MAX_MPDU] = { 0 };
+
+static uint8_t Rx_Buf[MAX_MPDU + 16 /* Add a little safety margin to the buffer,
+                                     * so that in the rare case, the message
+                                     * would be filled up to MAX_MPDU and some
+                                     * decoding functions would overrun, these
+                                     * decoding functions will just end up in
+                                     * a safe field of static zeros. */] = { 0 };
 
 static void LocalIAmHandler(
     uint8_t *service_request, uint16_t service_len, BACNET_ADDRESS *src)

ports/esp32/src/main.c

@@ -48,7 +48,15 @@ wifi_config_t wifi_config = {
 #define BACNET_LED 5
 
 uint8_t Handler_Transmit_Buffer[MAX_PDU] = { 0 };
-uint8_t Rx_Buf[MAX_MPDU] = { 0 };
+
+/** Static receive buffer, initialized with zeros by the C Library Startup Code. */
+
+uint8_t Rx_Buf[MAX_MPDU + 16 /* Add a little safety margin to the buffer,
+                              * so that in the rare case, the message
+                              * would be filled up to MAX_MPDU and some
+                              * decoding functions would overrun, these
+                              * decoding functions will just end up in
+                              * a safe field of static zeros. */] = { 0 };
 
 EventGroupHandle_t wifi_event_group;
 const static int CONNECTED_BIT = BIT0;
@@ -207,4 +215,4 @@ void app_main()
         NULL, /* Task input parameter */
         20, /* Priority of the task */
         NULL); /* Task handle. */
-}
+}

ports/linux/bip-init.c

@@ -331,7 +331,7 @@ uint16_t bip_receive(
     max = BIP_Socket;
     /* see if there is a packet for us */
     if (select(max + 1, &read_fds, NULL, NULL, &select_timeout) > 0) {
-        received_bytes = recvfrom(BIP_Socket, (char *)&npdu[0], max_npdu, 0,
+        received_bytes = recvfrom(max, (char *)&npdu[0], max_npdu, 0,
             (struct sockaddr *)&sin, &sin_len);
     } else {
         return 0;
@@ -348,6 +348,17 @@ uint16_t bip_receive(
     if (npdu[0] != BVLL_TYPE_BACNET_IP) {
         return 0;
     }
+    /* Erase up to 16 bytes after the received bytes as safety margin to
+     * ensure that the decoding functions will run into a 'safe field'
+     * of zero, if for any reason they would overrun, when parsing the
+     * message. */
+    max = (int)max_npdu - received_bytes;
+    if (max > 0) {
+        if (max > 16) {
+            max = 16;
+        }
+        memset(&npdu[received_bytes], 0, max);
+    }
     /* Data link layer addressing between B/IPv4 nodes consists of a 32-bit
        IPv4 address followed by a two-octet UDP port number (both of which
        shall be transmitted with the most significant octet first). This

ports/rx62n/bacnet.c

@@ -74,7 +74,15 @@ void bacnet_init(void)
     Send_I_Am(&Handler_Transmit_Buffer[0]);
 }
 
-static uint8_t PDUBuffer[MAX_MPDU];
+/** Static receive buffer, initialized with zeros by the C Library Startup Code. */
+
+static uint8_t PDUBuffer[MAX_MPDU + 16 /* Add a little safety margin to the buffer,
+                                        * so that in the rare case, the message
+                                        * would be filled up to MAX_MPDU and some
+                                        * decoding functions would overrun, these
+                                        * decoding functions will just end up in
+                                        * a safe field of static zeros. */];
+
 void bacnet_task(void)
 {
     uint16_t pdu_len;
@@ -110,7 +118,7 @@ void bacnet_task(void)
         dcc_timer_seconds(DCC_CYCLE_SECONDS);
     }
     /* handle the messaging */
-    pdu_len = datalink_receive(&src, &PDUBuffer[0], sizeof(PDUBuffer), 0);
+    pdu_len = datalink_receive(&src, &PDUBuffer[0], MAX_MPDU, 0);
     if (pdu_len) {
         npdu_handler(&src, &PDUBuffer[0], pdu_len);
     }

ports/stm32f10x/bacnet.c

@@ -79,7 +79,17 @@ void bacnet_init(void)
     Send_I_Am(&Handler_Transmit_Buffer[0]);
 }
 
-static uint8_t PDUBuffer[MAX_MPDU];
+/** Static receive buffer, initialized with zeros by the C Library Startup Code. */
+
+static uint8_t PDUBuffer[MAX_MPDU + 16 /* Add a little safety margin to the buffer,
+                                        * so that in the rare case, the message
+                                        * would be filled up to MAX_MPDU and some
+                                        * decoding functions would overrun, these
+                                        * decoding functions will just end up in
+                                        * a safe field of static zeros. */];
+
+/** BACnet task handling receiving and transmitting of messages.  */
+
 void bacnet_task(void)
 {
     uint16_t pdu_len;
@@ -123,7 +133,7 @@ void bacnet_task(void)
         dcc_timer_seconds(DCC_CYCLE_SECONDS);
     }
     /* handle the messaging */
-    pdu_len = datalink_receive(&src, &PDUBuffer[0], sizeof(PDUBuffer), 0);
+    pdu_len = datalink_receive(&src, &PDUBuffer[0], MAX_MPDU, 0);
     if (pdu_len) {
         npdu_handler(&src, &PDUBuffer[0], pdu_len);
     }

ports/win32/bip-init.c

@@ -484,7 +484,7 @@ uint16_t bip_receive(
     max = BIP_Socket;
     /* see if there is a packet for us */
     if (select(max + 1, &read_fds, NULL, NULL, &select_timeout) > 0) {
-        received_bytes = recvfrom(BIP_Socket, (char *)&npdu[0], max_npdu, 0,
+        received_bytes = recvfrom(max, (char *)&npdu[0], max_npdu, 0,
             (struct sockaddr *)&sin, &sin_len);
     } else {
         return 0;
@@ -501,6 +501,17 @@ uint16_t bip_receive(
     if (npdu[0] != BVLL_TYPE_BACNET_IP) {
         return 0;
     }
+    /* Erase up to 16 bytes after the received bytes as safety margin to
+     * ensure that the decoding functions will run into a 'safe field'
+     * of zero, if for any reason they would overrun, when parsing the
+     * message. */
+    max = (int)max_npdu - received_bytes;
+    if (max > 0) {
+        if (max > 16) {
+            max = 16;
+        }
+        memset(&npdu[received_bytes], 0, max);
+    }
     /* Data link layer addressing between B/IPv4 nodes consists of a 32-bit
        IPv4 address followed by a two-octet UDP port number (both of which
        shall be transmitted with the most significant octet first). This

ports/win32/main.c

@@ -59,8 +59,14 @@
 #include "bacnet/basic/object/bacfile.h"
 #endif
 
-/* buffer used for receive */
+/** Static receive buffer, initialized with zeros by the C Library Startup Code. */
-static uint8_t Rx_Buf[MAX_MPDU] = { 0 };
+
+static uint8_t Rx_Buf[MAX_MPDU + 16 /* Add a little safety margin to the buffer,
+                                     * so that in the rare case, the message
+                                     * would be filled up to MAX_MPDU and some
+                                     * decoding functions would overrun, these
+                                     * decoding functions will just end up in
+                                     * a safe field of static zeros. */] = { 0 };
 
 /* send a whois to see who is on the network */
 static bool Who_Is_Request = true;

src/bacnet/bacapp.c

@@ -926,10 +926,18 @@ bool bacapp_copy(BACNET_APPLICATION_DATA_VALUE *dest_value,
     return status;
 }
 
-/* returns the length of data between an opening tag and a closing tag.
+/**
-   Expects that the first octet contain the opening tag.
+ * @brief Returns the length of data between an opening tag and a closing tag.
-   Include a value property identifier for context specific data
+ * Expects that the first octet contain the opening tag.
-   such as the value received in a WriteProperty request */
+ * Include a value property identifier for context specific data
+ * such as the value received in a WriteProperty request.
+ *
+ * @param Pointer to the APDU buffer
+ * @param apdu_len_max Bytes valid in the buffer
+ * @param property ID of the propery to get the length for.
+ *
+ * @return Length in bytes or BACNET_STATUS_ERROR.
+ */
 int bacapp_data_len(
     uint8_t *apdu, unsigned apdu_len_max, BACNET_PROPERTY_ID property)
 {

src/bacnet/timestamp.c

@@ -37,6 +37,11 @@
 
 /** @file timestamp.c  Encode/Decode BACnet Timestamps  */
 
+/** Set the sequence number in a timestamp structure.
+ *
+ * @param dest  Pointer to the destination time stamp structure.
+ * @param sequenceNum  Sequence number to be set into the time stamp.
+ */
 void bacapp_timestamp_sequence_set(BACNET_TIMESTAMP *dest, uint16_t sequenceNum)
 {
     if (dest) {
@@ -45,6 +50,12 @@ void bacapp_timestamp_sequence_set(BACNET_TIMESTAMP *dest, uint16_t sequenceNum)
     }
 }
 
+/** Set a timestamp structure with the value given
+ * from a time structure.
+ *
+ * @param dest  Pointer to the destination time stamp structure.
+ * @param btime  Pointer to the BACNet time structure.
+ */
 void bacapp_timestamp_time_set(BACNET_TIMESTAMP *dest, BACNET_TIME *btime)
 {
     if (dest && btime) {
@@ -53,6 +64,12 @@ void bacapp_timestamp_time_set(BACNET_TIMESTAMP *dest, BACNET_TIME *btime)
     }
 }
 
+/** Set a timestamp structure with the value given
+ * from a date/time structure.
+ *
+ * @param dest  Pointer to the destination time stamp structure.
+ * @param bdateTime  Pointer to the BACNet date/time structure.
+ */
 void bacapp_timestamp_datetime_set(
     BACNET_TIMESTAMP *dest, BACNET_DATE_TIME *bdateTime)
 {
@@ -62,6 +79,11 @@ void bacapp_timestamp_datetime_set(
     }
 }
 
+/** Copy a timestamp depending of the tag it holds.
+ *
+ * @param dest  Pointer to the destination time stamp structure.
+ * @param src   Pointer to the source time stamp structure.
+ */
 void bacapp_timestamp_copy(BACNET_TIMESTAMP *dest, BACNET_TIMESTAMP *src)
 {
     if (dest && src) {
@@ -82,6 +104,14 @@ void bacapp_timestamp_copy(BACNET_TIMESTAMP *dest, BACNET_TIMESTAMP *src)
     }
 }
 
+/** Encode a time stamp.
+ *
+ * @param apdu  Pointer to the APDU buffer.
+ * @param value  Pointer to the variable that holds
+ *               the time stamp values to be encoded.
+ *
+ * @return Bytes encoded or 0 on error.
+ */
 int bacapp_encode_timestamp(uint8_t *apdu, BACNET_TIMESTAMP *value)
 {
     int len = 0; /* length of each encoding */
@@ -112,6 +142,17 @@ int bacapp_encode_timestamp(uint8_t *apdu, BACNET_TIMESTAMP *value)
     return len;
 }
 
+/** Encode a time stamp for the given tag
+ * number, using an opening and closing tag.
+ *
+ * @param apdu  Pointer to the APDU buffer.
+ * @param tag_number  The tag number that shall
+ *                    hold the time stamp.
+ * @param value  Pointer to the variable that holds
+ *               the time stamp values to be encoded.
+ *
+ * @return Bytes encoded or 0 on error.
+ */
 int bacapp_encode_context_timestamp(
     uint8_t *apdu, uint8_t tag_number, BACNET_TIMESTAMP *value)
 {
@@ -129,6 +170,14 @@ int bacapp_encode_context_timestamp(
     return apdu_len;
 }
 
+/** Decode a time stamp from the given buffer.
+ *
+ * @param apdu  Pointer to the APDU buffer.
+ * @param value  Pointer to the variable that shall
+ *               take the time stamp values.
+ *
+ * @return Bytes decoded or -1 on error.
+ */
 int bacapp_decode_timestamp(uint8_t *apdu, BACNET_TIMESTAMP *value)
 {
     int len = 0;
@@ -136,7 +185,7 @@ int bacapp_decode_timestamp(uint8_t *apdu, BACNET_TIMESTAMP *value)
     uint32_t len_value_type;
     BACNET_UNSIGNED_INTEGER unsigned_value;
 
-    if (apdu) {
+    if (apdu && value) {
         section_len = decode_tag_number_and_value(
             &apdu[len], &value->tag, &len_value_type);
 
@@ -184,6 +233,17 @@ int bacapp_decode_timestamp(uint8_t *apdu, BACNET_TIMESTAMP *value)
     return len;
 }
 
+/** Decode a time stamp and check for
+ * opening and closing tags.
+ *
+ * @param apdu  Pointer to the APDU buffer.
+ * @param tag_number  The tag number that shall
+ *                    hold the time stamp.
+ * @param value  Pointer to the variable that shall
+ *               take the time stamp values.
+ *
+ * @return Bytes decoded or -1 on error.
+ */
 int bacapp_decode_context_timestamp(
     uint8_t *apdu, uint8_t tag_number, BACNET_TIMESTAMP *value)
 {

src/bacnet/wpm.c

@@ -151,20 +151,25 @@ int wpm_decode_object_property(
                 (unsigned)(apdu_len - len), wp_data->object_property);
             len++;
 
-            /* copy application data, check max lengh */
+            if (imax != BACNET_STATUS_ERROR) {
-            if (imax > (apdu_len - len)) {
+                /* copy application data, check max lengh */
-                imax = (apdu_len - len);
+                if (imax > (apdu_len - len)) {
-            }
+                    imax = (apdu_len - len);
-            for (i = 0; i < imax; i++) {
+                }
-                wp_data->application_data[i] = apdu[len + i];
+                for (i = 0; i < imax; i++) {
-            }
+                    wp_data->application_data[i] = apdu[len + i];
-            wp_data->application_data_len = imax;
+                }
-            len += imax;
+                wp_data->application_data_len = imax;
-            if (len < apdu_len) {
+                len += imax;
-                len += decode_tag_number_and_value(
+                if (len < apdu_len) {
-                    &apdu[len], &tag_number, &len_value);
+                    len += decode_tag_number_and_value(
-                /* closing tag 2 */
+                        &apdu[len], &tag_number, &len_value);
-                if ((tag_number != 2) && (decode_is_closing_tag(&apdu[len - 1]))) {
+                    /* closing tag 2 */
+                    if ((tag_number != 2) && (decode_is_closing_tag(&apdu[len - 1]))) {
+                        wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
+                        return BACNET_STATUS_REJECT;
+                    }
+                } else {
                     wp_data->error_code = ERROR_CODE_REJECT_INVALID_TAG;
                     return BACNET_STATUS_REJECT;
                 }