Bugfix/validate-user-provided-file-object-paths (#1197)

* Fixed BACnet file object path name unintended path traversals by optionally restricting path name content with BACNET_FILE_PATH_RESTRICTED define.

* Added POSIX file path name checking for AtomicReadFile and AtomicWriteFile example applications. Prohibits use of relative and absolute file paths when BACNET_FILE_PATH_RESTRICTED is non-zero.

CHANGELOG.md

@@ -12,7 +12,7 @@ The git repositories are hosted at the following sites:
 * https://bacnet.sourceforge.net/
 * https://github.com/bacnet-stack/bacnet-stack/
 
-## [Unreleased] - 2026-01-03
+## [Unreleased] - 2026-01-05
 
 ### Security
 
@@ -26,9 +26,15 @@ The git repositories are hosted at the following sites:
   Fixed ubasic string variables to initialize with zeros.
   Fixed compile errors when UBASIC_DEBUG_STRINGVARIABLES is defined.
   Added ubasic string variables user accessor API and unit testing. (#1196)
+* Secured BACnet file object pathname received from BACnet AtomicWriteFile
+  or ReadFile service used without validation which was vulnerable to
+  directory traversal attacks. (#1197)
 
 ### Added
 
+* Added file path name checking for AtomicReadFile and AtomicWriteFile
+  example applications. Prohibits use of relative and absolute file paths
+  when BACNET_FILE_PATH_RESTRICTED is defined non-zero. (#1197)
 * Added API and optional properties to basic load control object example
   Refactored BACnetShedLevel encoding, decoding, and printing into separate
   file. Added BACnetShedLevel validation testing. (#1187)

apps/readfile/main.c

@@ -286,6 +286,10 @@ int main(int argc, char *argv[])
     /* decode the command line parameters */
     Target_Device_Object_Instance = strtol(argv[1], NULL, 0);
     Target_File_Object_Instance = strtol(argv[2], NULL, 0);
+    if (!filename_path_valid(argv[3])) {
+        fprintf(stderr, "Invalid file path: %s\n", argv[3]);
+        return 1;
+    }
     Local_File_Name = argv[3];
     if (Target_Device_Object_Instance > BACNET_MAX_INSTANCE) {
         fprintf(

apps/writefile/main.c

@@ -159,6 +159,10 @@ int main(int argc, char *argv[])
     /* decode the command line parameters */
     Target_Device_Object_Instance = strtol(argv[1], NULL, 0);
     Target_File_Object_Instance = strtol(argv[2], NULL, 0);
+    if (!filename_path_valid(argv[3])) {
+        fprintf(stderr, "Invalid file path: %s\n", argv[3]);
+        return 1;
+    }
     Local_File_Name = argv[3];
     if (Target_Device_Object_Instance > BACNET_MAX_INSTANCE) {
         fprintf(

ports/posix/bacfile-posix.c

@@ -13,11 +13,14 @@
 #include <string.h>
 /* BACnet Stack defines - first */
 #include "bacnet/bacdef.h"
-#include "bacnet/basic/sys/debug.h"
 #include "bacnet/basic/object/bacfile.h"
+#include "bacnet/basic/sys/debug.h"
+#include "bacnet/basic/sys/filename.h"
+/* me! */
+#include "bacfile-posix.h"
 
-#ifndef FILE_RECORD_SIZE
-#define FILE_RECORD_SIZE MAX_OCTET_STRING_BYTES
+#ifndef BACNET_FILE_POSIX_RECORD_SIZE
+#define BACNET_FILE_POSIX_RECORD_SIZE MAX_OCTET_STRING_BYTES
 #endif
 
 /**
@@ -50,7 +53,7 @@ size_t bacfile_posix_file_size(const char *pathname)
     long file_position = 0;
     size_t file_size = 0;
 
-    if (pathname) {
+    if (filename_path_valid(pathname)) {
         pFile = fopen(pathname, "rb");
         if (pFile) {
             file_position = fsize(pFile);
@@ -100,7 +103,7 @@ size_t bacfile_posix_read_stream_data(
     FILE *pFile = NULL;
     size_t len = 0;
 
-    if (pathname) {
+    if (filename_path_valid(pathname)) {
         pFile = fopen(pathname, "rb");
         if (pFile) {
             (void)fseek(pFile, fileStartPosition, SEEK_SET);
@@ -131,7 +134,7 @@ size_t bacfile_posix_write_stream_data(
     size_t bytes_written = 0;
     FILE *pFile = NULL;
 
-    if (pathname) {
+    if (filename_path_valid(pathname)) {
         if (fileStartPosition == 0) {
             /* open the file as a clean slate when starting at 0 */
             pFile = fopen(pathname, "wb");
@@ -177,11 +180,11 @@ bool bacfile_posix_write_record_data(
     bool status = false;
     FILE *pFile = NULL;
     uint32_t i = 0;
-    char dummy_data[FILE_RECORD_SIZE];
+    char dummy_data[BACNET_FILE_POSIX_RECORD_SIZE];
     const char *pData = NULL;
     size_t fileSeekRecord = 0;
 
-    if (pathname) {
+    if (filename_path_valid(pathname)) {
         if (fileStartRecord == 0) {
             /* open the file as a clean slate when starting at 0 */
             pFile = fopen(pathname, "wb");
@@ -238,11 +241,11 @@ bool bacfile_posix_read_record_data(
     bool status = false;
     FILE *pFile = NULL;
     uint32_t i = 0;
-    char dummy_data[FILE_RECORD_SIZE] = { 0 };
+    char dummy_data[BACNET_FILE_POSIX_RECORD_SIZE] = { 0 };
     const char *pData = NULL;
     size_t fileSeekRecord = 0;
 
-    if (pathname) {
+    if (filename_path_valid(pathname)) {
         pFile = fopen(pathname, "rb");
         if (pFile) {
             fileSeekRecord = fileStartRecord + fileIndexRecord;

src/bacnet/basic/object/bacfile.c

@@ -29,9 +29,6 @@
 #include "bacnet/basic/sys/keylist.h"
 #include "bacnet/basic/tsm/tsm.h"
 
-#ifndef FILE_RECORD_SIZE
-#define FILE_RECORD_SIZE MAX_OCTET_STRING_BYTES
-#endif
 struct object_data {
     char *Object_Name;
     char *Pathname;

src/bacnet/basic/sys/filename.c

@@ -1,14 +1,25 @@
 /**
  * @file
- * @brief Function for filename manipulation
+ * @brief Function for filename and path manipulation and validation
  * @author Steve Karg <skarg@users.sourceforge.net>
  * @date 2007
  * @copyright SPDX-License-Identifier: GPL-2.0-or-later WITH GCC-exception-2.0
  */
 #include <stdio.h>
 #include <string.h>
+#include "bacnet/basic/sys/debug.h"
 #include "bacnet/basic/sys/filename.h"
 
+/* restrict file paths */
+#ifndef BACNET_FILE_PATH_RESTRICTED
+#define BACNET_FILE_PATH_RESTRICTED 1
+#endif
+
+/**
+ * @brief Remove path from filename
+ * @param filename_in - input filename with path
+ * @return filename without path
+ */
 const char *filename_remove_path(const char *filename_in)
 {
     const char *filename_out = filename_in;
@@ -30,3 +41,57 @@ const char *filename_remove_path(const char *filename_in)
 
     return filename_out;
 }
+
+/**
+ * @brief Validate if pathname is valid by checking for patterns
+ *  such as relative paths and absolute paths which are prohibited.
+ * @param pathname Path to validate
+ * @return true if valid, false if not
+ */
+bool filename_path_valid(const char *pathname)
+{
+    int path_len;
+
+    if (!pathname) {
+        return false;
+    }
+    if (pathname[0] == 0) {
+        return false;
+    }
+#if BACNET_FILE_PATH_RESTRICTED
+    /* check for relative directory patterns */
+    if (strstr(pathname, "..") != NULL) {
+        debug_printf_stderr("Relative paths are prohibited: %s\n", pathname);
+        return false;
+    }
+    /* check for absolute paths */
+    if (pathname[0] == '/') {
+        debug_printf_stderr("Absolute paths are prohibited: %s\n", pathname);
+        return false;
+    }
+    /* check for Windows drive letters (should be relative paths only) */
+    path_len = strlen(pathname);
+    if (path_len >= 2 && pathname[1] == ':') {
+        debug_printf_stderr(
+            "Windows drive letters are prohibited: %s\n", pathname);
+        return false;
+    }
+
+    /* check for consecutive path separators */
+    if (strstr(pathname, "//") != NULL || strstr(pathname, "\\\\") != NULL) {
+        debug_printf_stderr(
+            "Consecutive path separators are prohibited: %s\n", pathname);
+        return false;
+    }
+
+    /* check for path components that are just dots */
+    if (strstr(pathname, "/./") != NULL || strstr(pathname, "\\./") != NULL ||
+        strstr(pathname, "/.\\") != NULL || strstr(pathname, "\\.\\") != NULL) {
+        debug_printf_stderr(
+            "Current directory references are prohibited: %s\n", pathname);
+        return false;
+    }
+#endif
+
+    return true;
+}

src/bacnet/basic/sys/filename.h

@@ -9,6 +9,10 @@
 #define BACNET_SYS_FILENAME_H
 /* BACnet Stack defines - first */
 #include "bacnet/bacdef.h"
+/* standard includes */
+#include <stdbool.h>
+#include <stddef.h>
+#include <stdint.h>
 
 #ifdef __cplusplus
 extern "C" {
@@ -16,6 +20,8 @@ extern "C" {
 
 BACNET_STACK_EXPORT
 const char *filename_remove_path(const char *filename_in);
+BACNET_STACK_EXPORT
+bool filename_path_valid(const char *pathname);
 
 #ifdef __cplusplus
 }

test/bacnet/basic/object/netport/CMakeLists.txt

@@ -85,6 +85,7 @@ add_executable(${PROJECT_NAME}
     ${SRC_DIR}/bacnet/basic/sys/debug.c
     ${SRC_DIR}/bacnet/basic/sys/bigend.c
     ${SRC_DIR}/bacnet/basic/sys/days.c
+    ${SRC_DIR}/bacnet/basic/sys/filename.c
     ${SRC_DIR}/bacnet/basic/sys/keylist.c
     ${SRC_DIR}/bacnet/datalink/bvlc.c
     ${SRC_DIR}/bacnet/datalink/bvlc6.c

test/bacnet/basic/sys/filename/CMakeLists.txt

@@ -34,6 +34,7 @@ add_executable(${PROJECT_NAME}
     # File(s) under test
     ${SRC_DIR}/bacnet/basic/sys/filename.c
     # Support files and stubs (pathname alphabetical)
+    ${SRC_DIR}/bacnet/basic/sys/debug.c
     # Test and test library files
     ./src/main.c
     ${ZTST_DIR}/ztest_mock.c

test/bacnet/basic/sys/filename/src/main.c

@@ -27,6 +27,7 @@ static void testFilename(void)
     const char *data3 = "c:\\Program Files\\Christopher\\run.exe";
     const char *data4 = "//Mary/data/run";
     const char *data5 = "bin\\run";
+    const char *data6 = "run.exe";
     const char *filename = NULL;
 
     filename = filename_remove_path(data1);
@@ -39,9 +40,53 @@ static void testFilename(void)
     zassert_equal(strcmp("run", filename), 0, NULL);
     filename = filename_remove_path(data5);
     zassert_equal(strcmp("run", filename), 0, NULL);
+    filename = filename_remove_path(data6);
+    zassert_equal(strcmp("run.exe", filename), 0, NULL);
 
     return;
 }
+
+#if defined(CONFIG_ZTEST_NEW_API)
+ZTEST(filename_tests, testFilenameValid)
+#else
+static void testFilenameValid(void)
+#endif
+{
+    const char *data0 = "";
+    const char *data1 = "c:\\Joshua\\run";
+    const char *data2 = "/home/Anna/run";
+    const char *data3 = "c:\\Program Files\\Christopher\\run.exe";
+    const char *data4 = "//Mary/data/run";
+    const char *data5 = "bin\\\\run";
+    const char *data6 = "bin/./run";
+    const char *data7 = "bin/../run";
+    const char *data_valid = "certs/mycert.pem";
+    bool valid = false;
+
+    valid = filename_path_valid(NULL);
+    zassert_false(valid, NULL);
+    valid = filename_path_valid(data0);
+    zassert_false(valid, NULL);
+    valid = filename_path_valid(data1);
+    zassert_false(valid, NULL);
+    valid = filename_path_valid(data2);
+    zassert_false(valid, NULL);
+    valid = filename_path_valid(data3);
+    zassert_false(valid, NULL);
+    valid = filename_path_valid(data4);
+    zassert_false(valid, NULL);
+    valid = filename_path_valid(data5);
+    zassert_false(valid, NULL);
+    valid = filename_path_valid(data6);
+    zassert_false(valid, NULL);
+    valid = filename_path_valid(data7);
+    zassert_false(valid, NULL);
+    valid = filename_path_valid(data_valid);
+    zassert_true(valid, NULL);
+
+    return;
+}
+
 /**
  * @}
  */
@@ -51,7 +96,9 @@ ZTEST_SUITE(filename_tests, NULL, NULL, NULL, NULL, NULL);
 #else
 void test_main(void)
 {
-    ztest_test_suite(filename_tests, ztest_unit_test(testFilename));
+    ztest_test_suite(
+        filename_tests, ztest_unit_test(testFilename),
+        ztest_unit_test(testFilenameValid));
 
     ztest_run_test_suite(filename_tests);
 }

test/bacnet/datalink/bsc-datalink/CMakeLists.txt

@@ -167,6 +167,7 @@ target_sources(${PROJECT_NAME} PRIVATE
   ${SRC_DIR}/bacnet/basic/sys/days.c
   ${SRC_DIR}/bacnet/basic/sys/debug.c
   ${SRC_DIR}/bacnet/basic/sys/fifo.c
+  ${SRC_DIR}/bacnet/basic/sys/filename.c
   ${SRC_DIR}/bacnet/basic/sys/keylist.c
   ${SRC_DIR}/bacnet/basic/sys/mstimer.c
   ${SRC_DIR}/bacnet/access_rule.c