Summary I found a server-side denial-of-service issue in bacnet-stack 1.6.0 affecting the Structured View object implementation. The issue is reachable through the standard BACnet/IP `WriteProperty` path and is exposed by the official demo server (`apps/server/main.c`). A remote, unauthenticated client can send a single `WriteProperty` request that targets: - Object Type: `OBJECT_STRUCTURED_VIEW` - Object Instance: `1` (default demo object) - Property: `PROP_SUBORDINATE_LIST` - Array Index: `0` - Value: a large `Unsigned`, e.g. `100000` For this property, array index `0` is interpreted as the array cardinality rather than an ordinary element write. The implementation therefore converts one small network request into a large, synchronous, heap-backed expansion loop in the request-processing path. As a result, the server can spend a long time inside the write handler, with high CPU usage, steadily increasing memory consumption, and no timely ACK/Error response to the client. Repeated requests can sustain a practical device-level DoS, and memory pressure may additionally lead to OOM termination depending on the runtime environment. --- Version - Version: `v1.6.0` - Commit: `a513079` --- Impact Security impact A remote unauthenticated attacker on the BACnet/IP network can send a single valid `WriteProperty` request that causes the server to perform an unbounded synchronous resize of the Structured View subordinate list. Practical effects: - CPU exhaustion: the server remains busy in the resize loop. - Memory growth: the server allocates and inserts a large number of subordinate-list elements. - Request-path blockage: the server does not promptly return from `Device_Write_Property()`, so the client commonly observes APDU timeout. - Effective denial of service: while the server is trapped in this work, it becomes unable or severely delayed in processing normal BACnet traffic. This is a network-reachable, unauthenticated, protocol-level DoS on a server path that is enabled in the official demo and implemented in the library itself. It does not require malformed parser fuzzing, local access, or application misuse. --- Affected path Dispatch path - `src/bacnet/basic/service/h_wp.c` - `handler_write_property()` - `src/bacnet/basic/object/device.c` - dispatch table entry for `OBJECT_STRUCTURED_VIEW -> Structured_View_Write_Property` Root-cause path - `src/bacnet/bacdcode.c` - generic BACnet array-write helper (`bacnet_array_write_resizable()` / `bacnet_array_write()` path) - `src/bacnet/basic/object/structured_view.c` - `Structured_View_Write_Property()` - `Structured_View_Subordinate_List_Member_Write()` - `Structured_View_Subordinate_List_Resize()` - `Subordinate_List_Element_Add()` - `src/bacnet/basic/sys/keylist.c` - `CheckArraySize()` - `Keylist_Data_Add()` --- Root cause 1. The property is intentionally writable In `src/bacnet/basic/object/structured_view.c`, `PROP_SUBORDINATE_LIST` is included in the Structured View writable property list. This means the write path is not accidentally exposed; it is part of the supported object behavior. 2. The official server demo exposes the vulnerable object by default In `apps/server/main.c`, the demo server creates dynamic example objects and configures an example subordinate list for the first Structured View. `Structured_View_Update()` links the `Lighting_Subordinate` array and calls `Structured_View_Subordinate_List_Set(instance, Lighting_Subordinate)`. As a result, the demo server starts with a real Structured View object that already has a populated subordinate list. This matters because the issue is directly reachable in the stock demo configuration using the ordinary `WriteProperty` service. 3. The generic BACnetARRAY write path treats array index 0 as cardinality In the generic array-write helper in `src/bacnet/bacdcode.c`, array index `0` is treated specially: it represents the number of elements in the array. Therefore, a request such as: - property = `subordinate-list` - array-index = `0` - value = `Unsigned(100000)` is not interpreted as “write one subordinate-list element.” Instead, it is interpreted as “set the subordinate-list size to 100000.” This is the trust boundary violation: a small network-supplied scalar becomes a large internal work budget. 4. Structured View directly maps attacker-controlled cardinality into resize work `Structured_View_Subordinate_List_Member_Write()` in `src/bacnet/basic/object/structured_view.c` explicitly handles array index `0` as the array size case and directly calls: - `Structured_View_Subordinate_List_Resize(pObject, array_size)` There is no object-level upper bound enforced before this handoff. 5. The resize loop is synchronous and unbounded `Structured_View_Subordinate_List_Resize()` obtains the current list size and then, when `new_array_size > old_array_size`, executes a linear loop: - initialize `key = old_array_size` - while `key < new_array_size` - call `Subordinate_List_Element_Add(...)` - increment `key` The loop only stops when the requested size is reached or allocation fails. There is no maximum cardinality check, no per-request growth cap, no quota, and no early rejection based on computational cost. 6. Each iteration performs additional allocator/container work The growth operation is not a cheap metadata change. Each append flows into the keylist container implementation in `src/bacnet/basic/sys/keylist.c`, including `CheckArraySize()` and `Keylist_Data_Add()`, which in turn allocate and insert nodes. This converts a single BACnet write into many heap allocations and container operations. 7. The request handler is blocked until the write returns In `src/bacnet/basic/service/h_wp.c`, `handler_write_property()` decodes the request and then calls `Device_Write_Property(&wp_data)`. Only after that call returns does the handler build and send a simple ACK or an error APDU. Therefore, if the server is trapped in the Structured View resize loop, the requester sees: - no timely reply, often ending in `APDU Timeout!` - and the server remains busy in the same synchronous request-processing path. --- Reproduction 1. Build ```bash cmake -S . -B build-asan \ -DCMAKE_C_COMPILER=clang \ -DCMAKE_BUILD_TYPE=RelWithDebInfo \ -DCMAKE_C_FLAGS="-O1 -g -fno-omit-frame-pointer -fsanitize=address,undefined" \ -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined" cmake --build build-asan -j"$(nproc)" --target server writeprop ``` 2. Start the server ```bash BACNET_IFACE=lo BACNET_IP_PORT=47844 ./build-asan/server 904 ``` 3. Trigger with the official client ```bash BACNET_IFACE=lo ./build-asan/writeprop \ 904 structured-view 1 subordinate-list 16 0 2 100000 \ --mac 127.0.0.1:47844 ``` This request means: - device instance = `904` - object = `structured-view 1` - property = `subordinate-list` - priority = `16` - array index = `0` - application data type = `2` (`Unsigned`) - value = `100000` 4. External success indicators Client side ```bash Error: APDU Timeout! ``` This timeout is expected here because the server spends a long time inside the resize path and does not promptly return to the point where it would send ACK/Error. --- GDB-assisted confirmation 1. Debug build ```bash cmake -S . -B build-gdb-asan \ -DCMAKE_C_COMPILER=clang \ -DCMAKE_BUILD_TYPE=Debug \ -DCMAKE_C_FLAGS="-O0 -g3 -fno-omit-frame-pointer -fsanitize=address,undefined" \ -DCMAKE_EXE_LINKER_FLAGS="-fsanitize=address,undefined" cmake --build build-gdb-asan -j"$(nproc)" --target server writeprop ``` 2. Launch under GDB ```bash export BACNET_IFACE=lo export BACNET_IP_PORT=47847 gdb -q --args ./build-gdb-asan/server 907 ``` Suggested breakpoints: ```gdb set pagination off b src/bacnet/basic/service/h_wp.c:120 b src/bacnet/basic/object/structured_view.c:711 b src/bacnet/basic/object/structured_view.c:640 b src/bacnet/basic/sys/keylist.c:78 run ``` 3. Key observations A. The request reaches the real WriteProperty path At `h_wp.c`, the decoded request data is: ```bash wp_data.object_type = OBJECT_STRUCTURED_VIEW wp_data.object_instance = 1 wp_data.object_property = PROP_SUBORDINATE_LIST wp_data.priority = 16 wp_data.array_index = 0 wp_data.application_data_len = 4 application_data = 23 01 86 a0 ``` `23 01 86 a0` is a BACnet application `Unsigned` encoding for `100000`. B. The object-specific writer receives attacker-controlled target cardinality At `Structured_View_Subordinate_List_Member_Write()`: ```bash object_instance = 1 array_index = 0 array_size = 100000 apdu_size = 4 apdu = 23 01 86 a0 ``` This confirms that the request is no longer a normal element write; it has become a set-array-size operation. C. The server attempts a very large resize from a small default list At `Structured_View_Subordinate_List_Resize()`: ```bash old_array_size = 7 new_array_size = 100000 key = 0 Structured_View_Subordinate_List_Size(pObject) = 7 ``` The demo server starts from a subordinate list of size `7`, then attempts to expand it to `100000`. D. The growth is iterative, not rejected up front Early stack snapshot in `keylist.c`: ```bash 0 CheckArraySize 1 Keylist_Data_Add 2 Subordinate_List_Element_Add 3 Structured_View_Subordinate_List_Resize 4 Structured_View_Subordinate_List_Member_Write 5 bacnet_array_write_resizable 6 bacnet_array_write 7 Structured_View_Write_Property ``` Representative locals: ```bash old_array_size = 7 key = 8 error_code = ERROR_CODE_SUCCESS ``` This proves the operation has already entered the per-element expansion path. E. The loop continues deeply into allocator/container work After letting the process run and interrupting it later, the stack remains in the same expansion chain, with large progress values such as: ```bash key = 58520 old_array_size = 7 new_array_size = 100000 error_code = ERROR_CODE_SUCCESS ``` This shows there is no early clamp, no cost-based rejection, and no fast-fail guard for oversized cardinality requests. Result analysis The defect is that a network-supplied BACnetARRAY cardinality is trusted and translated into a large, synchronous resize operation in the server thread. Operationally, the effect is: 1. the malicious request is accepted and decoded normally; 2. the generic array-write logic interprets array index `0` as array size; 3. Structured View forwards that size directly into a linear resize loop; 4. the resize loop performs repeated allocations and keylist insertions; 5. the write handler does not return in time, so the client times out; 6. the server consumes CPU and memory and becomes effectively unavailable. --- Fix suggestion Design goal The core problem is not that resize exists; it is that a remotely writable array-size field is unconstrained. Correct fix should: 1. define a maximum allowed subordinate-list cardinality; 2. reject oversized index-0 writes before entering the growth loop; 3. return a normal BACnet property error such as `ERROR_CODE_VALUE_OUT_OF_RANGE`. Proposed patch The following patch is a minimal library-level fix for `structured_view.c`. It adds a compile-time cardinality cap and rejects oversized `subordinate-list[0]` writes before the server enters the expensive expansion loop. ```diff diff --git a/src/bacnet/basic/object/structured_view.c b/src/bacnet/basic/object/structured_view.c --- a/src/bacnet/basic/object/structured_view.c +++ b/src/bacnet/basic/object/structured_view.c @@ static const int32_t Writable_Properties[] = { / unordered list of writable properties / PROP_NODE_TYPE, PROP_DEFAULT_SUBORDINATE_RELATIONSHIP, PROP_REPRESENTS, PROP_SUBORDINATE_LIST, PROP_SUBORDINATE_ANNOTATIONS, PROP_SUBORDINATE_NODE_TYPES, PROP_SUBORDINATE_RELATIONSHIPS, PROP_OBJECT_NAME, PROP_DESCRIPTION, -1 }; + +/ + Remotely writable BACnetARRAY element 0 encodes the number of elements in + the subordinate list. Without a guard, a single WriteProperty request can + trigger a very large synchronous expansion loop in the server path. + + Tune this value as needed for deployment requirements. + / +#ifndef STRUCTURED_VIEW_SUBORDINATE_LIST_MAX +#define STRUCTURED_VIEW_SUBORDINATE_LIST_MAX 1024U +#endif + +static bool Structured_View_Subordinate_List_Size_Allowed( + BACNET_UNSIGNED_INTEGER new_array_size) +{ + return (new_array_size <= STRUCTURED_VIEW_SUBORDINATE_LIST_MAX); +} @@ static BACNET_ERROR_CODE Structured_View_Subordinate_List_Resize( struct object_data *pObject, BACNET_UNSIGNED_INTEGER new_array_size) { BACNET_ERROR_CODE error_code = ERROR_CODE_SUCCESS; BACNET_SUBORDINATE_DATA *element = NULL; BACNET_UNSIGNED_INTEGER old_array_size = 0; KEY key = 0; old_array_size = Structured_View_Subordinate_List_Size(pObject); + + if (!Structured_View_Subordinate_List_Size_Allowed(new_array_size)) { + return ERROR_CODE_VALUE_OUT_OF_RANGE; + } + / Array element zero is the number of elements in the list. / if (new_array_size < old_array_size) { / free the elements at the tail of the list / key = new_array_size; while (key < old_array_size) { @@ static BACNET_ERROR_CODE Structured_View_Subordinate_List_Member_Write( uint32_t object_instance, BACNET_ARRAY_INDEX array_index, BACNET_UNSIGNED_INTEGER array_size, uint8_t *apdu, size_t apdu_size) { BACNET_ERROR_CODE error_code = ERROR_CODE_UNKNOWN_OBJECT; BACNET_DEVICE_OBJECT_REFERENCE reference = { 0 }; BACNET_SUBORDINATE_DATA *element = NULL; int len = 0; struct object_data *pObject; pObject = Keylist_Data(Object_List, object_instance); if (pObject) { if (array_index == 0) { / Array element zero is the number of elements in the list. / + if (!Structured_View_Subordinate_List_Size_Allowed(array_size)) { + return ERROR_CODE_VALUE_OUT_OF_RANGE; + } error_code = Structured_View_Subordinate_List_Resize(pObject, array_size); } else { array_index--; / array index is 1..N, but we want 0..(N-1) */ len = bacnet_device_object_reference_decode( apdu, apdu_size, &reference); ``` --- Remediation Applied the patch (thanks!) to structured view and command objects in PR #1437.