Bugfix/codeql initial fixes (#195)

* Fix to BVLC6 bounds checking

* Convert MSTP capture to use datetime library

* Convert timesync app to use datetime and mstimer library

* fix workflow per warning

* fix BSD mstimer init function name

Co-authored-by: Steve Karg <skarg@users.sourceforge.net>

.github/workflows/codeql-analysis.yml

@@ -3,7 +3,7 @@ name: CodeQL
 on:
   push:
     branches:
-      - master
+      - '*'
   pull_request:
     branches:
       - '*'

apps/mstpcap/Makefile

@@ -8,13 +8,16 @@ TARGET = mstpcap
 SRCS = main.c \
 	${BACNET_PORT_DIR}/rs485.c \
 	${BACNET_PORT_DIR}/mstimer-init.c \
+	${BACNET_PORT_DIR}/datetime-init.c \
 	${BACNET_SRC_DIR}/bacnet/bacdcode.c \
 	${BACNET_SRC_DIR}/bacnet/bacint.c \
 	${BACNET_SRC_DIR}/bacnet/bacreal.c \
 	${BACNET_SRC_DIR}/bacnet/bacstr.c \
 	${BACNET_SRC_DIR}/bacnet/iam.c \
+	${BACNET_SRC_DIR}/bacnet/datetime.c \
 	${BACNET_SRC_DIR}/bacnet/indtext.c \
 	${BACNET_SRC_DIR}/bacnet/npdu.c \
+	${BACNET_SRC_DIR}/bacnet/basic/sys/days.c \
 	${BACNET_SRC_DIR}/bacnet/basic/sys/debug.c \
 	${BACNET_SRC_DIR}/bacnet/basic/sys/fifo.c \
 	${BACNET_SRC_DIR}/bacnet/basic/sys/filename.c \

apps/mstpcap/main.c

@@ -42,6 +42,7 @@
 #include "bacnet/bytes.h"
 #include "bacnet/iam.h"
 #include "bacnet/version.h"
+#include "bacnet/datetime.h"
 /* basic datalink, timer, and filename */
 #include "bacnet/datalink/dlmstp.h"
 #include "bacnet/basic/sys/mstimer.h"
@@ -532,15 +533,14 @@ static size_t data_write_header(
 
 static void filename_create(char *filename)
 {
-    time_t my_time;
-    struct tm *today;
+    BACNET_DATE bdate;
+    BACNET_TIME btime;
 
     if (filename) {
-        my_time = time(NULL);
-        today = localtime(&my_time);
+        datetime_local(&bdate, &btime, NULL, NULL);
         sprintf(filename, "mstp_%04d%02d%02d%02d%02d%02d.cap",
-            1900 + today->tm_year, 1 + today->tm_mon, today->tm_mday,
-            today->tm_hour, today->tm_min, today->tm_sec);
+            (int)bdate.year, (int)bdate.month, (int)bdate.day,
+            (int)btime.hour, (int)btime.min, (int)btime.sec);
     }
 }
 

apps/timesync/main.c

@@ -29,7 +29,6 @@
 #include <stdio.h>
 #include <stdlib.h>
 #include <ctype.h>
-#include <time.h> /* for time */
 #include <errno.h>
 #include "bacnet/basic/binding/address.h"
 #include "bacnet/bactext.h"
@@ -37,12 +36,14 @@
 #include "bacnet/bacdef.h"
 #include "bacnet/npdu.h"
 #include "bacnet/apdu.h"
+#include "bacnet/datetime.h"
 #include "bacnet/basic/object/device.h"
 #include "bacport.h"
 #include "bacnet/datalink/datalink.h"
 #include "bacnet/timesync.h"
 #include "bacnet/version.h"
 /* some demo stuff needed */
+#include "bacnet/basic/sys/mstimer.h"
 #include "bacnet/basic/sys/filename.h"
 #include "bacnet/basic/services.h"
 #include "bacnet/basic/services.h"
@@ -152,15 +153,10 @@ int main(int argc, char *argv[])
 {
     BACNET_ADDRESS src = { 0 }; /* address where message came from */
     uint16_t pdu_len = 0;
-    unsigned timeout = 100; /* milliseconds */
-    time_t elapsed_seconds = 0;
-    time_t last_seconds = 0;
-    time_t current_seconds = 0;
-    time_t timeout_seconds = 0;
-    time_t rawtime;
-    struct tm *my_time;
+    const unsigned timeout = 100; /* milliseconds */
     BACNET_DATE bdate;
     BACNET_TIME btime;
+    struct mstimer apdu_timer;
     long dnet = -1;
     BACNET_MAC_ADDRESS mac = { 0 };
     BACNET_MAC_ADDRESS adr = { 0 };
@@ -246,40 +242,25 @@ int main(int argc, char *argv[])
     Init_Service_Handlers();
     dlenv_init();
     atexit(datalink_cleanup);
-    /* configure the timeout values */
-    last_seconds = time(NULL);
-    timeout_seconds = apdu_timeout() / 1000;
+    mstimer_init();
     /* send the request */
-    time(&rawtime);
-    my_time = localtime(&rawtime);
-    bdate.year = my_time->tm_year + 1900;
-    bdate.month = my_time->tm_mon + 1;
-    bdate.day = my_time->tm_mday;
-    bdate.wday = my_time->tm_wday ? my_time->tm_wday : 7;
-    btime.hour = my_time->tm_hour;
-    btime.min = my_time->tm_min;
-    btime.sec = my_time->tm_sec;
-    btime.hundredths = 0;
+    datetime_local(&bdate, &btime, NULL, NULL);
     Send_TimeSync_Remote(&dest, &bdate, &btime);
+    mstimer_set(&apdu_timer, apdu_timeout());
     /* loop forever - not necessary for time sync, but we can watch */
     for (;;) {
-        /* increment timer - exit if timed out */
-        current_seconds = time(NULL);
         /* returns 0 bytes on timeout */
         pdu_len = datalink_receive(&src, &Rx_Buf[0], MAX_MPDU, timeout);
         /* process */
         if (pdu_len) {
             npdu_handler(&src, &Rx_Buf[0], pdu_len);
         }
-        if (Error_Detected)
-            break;
-        /* increment timer - exit if timed out */
-        elapsed_seconds += (current_seconds - last_seconds);
-        if (elapsed_seconds > timeout_seconds) {
+        if (Error_Detected) {
+            break;
+        }
+        if (mstimer_expired(&apdu_timer)) {
             break;
         }
-        /* keep track of time for next check */
-        last_seconds = current_seconds;
     }
 
     return 0;

ports/bsd/mstimer-init.c

@@ -84,7 +84,7 @@ unsigned long mstimer_now(void)
 /**
  * @brief Initialization for timer
  */
-void timer_init(void)
+void mstimer_init(void)
 {
 #ifdef __MACH__
     clock_serv_t cclock;

src/bacnet/datalink/bvlc6.c

@@ -297,7 +297,7 @@ int bvlc6_encode_original_broadcast(uint8_t *pdu,
             pdu, pdu_size, BVLC6_ORIGINAL_BROADCAST_NPDU, length);
         if (bytes_encoded == 4) {
             encode_unsigned24(&pdu[4], vmac);
-            if (npdu && length) {
+            if (npdu && npdu_len) {
                 for (i = 0; i < npdu_len; i++) {
                     pdu[7 + i] = npdu[i];
                 }