Skip to content

Bound State_Text resize in state_name_list_write_resizable() - #1518

Merged
skarg merged 1 commit into
masterfrom
bugfix/state-name-list-resize-bound
Sep 28, 2026
Merged

skarg merged 1 commit into
masterfrom
bugfix/state-name-list-resize-bound

Conversation

@skarg

@skarg skarg commented Sep 28, 2026

Copy link
Copy Markdown
Collaborator

A WriteProperty of State_Text[0] (array size) or State_Text[N] to any multistate object (MSV, MSO, MSI) grew the keylist one sorted insert at a time without an upper bound. A single request with a large size blocked the single-threaded server loop for a very long time and could exhaust memory (same class as GHSA-hg85-pmm3-jfcf for Structured View).

  • Reject a new size of 0 or above BACNET_STATE_NAME_LIST_MAX (default 255) with VALUE_OUT_OF_RANGE; the list is left unchanged.
  • Reject an auto-expanding element write beyond the maximum with INVALID_ARRAY_INDEX.
  • Initialize new elements to an empty string instead of NULL so they can always be read back.
  • Unit test updated and extended.

A WriteProperty of State_Text[0] (array size) or State_Text[N] to any
multistate object (MSV, MSO, MSI) grew the keylist one sorted insert at
a time without an upper bound. A single request with a large size
blocked the single-threaded server loop for a very long time and could
exhaust memory (same class as GHSA-hg85-pmm3-jfcf for Structured View).

- Reject a new size of 0 or above BACNET_STATE_NAME_LIST_MAX (default
  255) with VALUE_OUT_OF_RANGE; the list is left unchanged.
- Reject an auto-expanding element write beyond the maximum with
  INVALID_ARRAY_INDEX.
- Initialize new elements to an empty string instead of NULL so they
  can always be read back.
- Unit test updated and extended.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟢 Approval recommended

No unresolved issues were identified.

Review effort: Lite
Findings: None

What changed in this PR

Limits resizable State_Text lists to prevent excessive allocation and initializes new entries as readable empty strings.

Changes:

  • Adds a configurable maximum of 255 entries.
  • Rejects invalid resize and out-of-range writes.
  • Extends tests for bounds and initialization behavior.
File Description
test/​bacnet/​basic/​sys/​state_name/​src/​main.c Tests resizing, limits, and empty entries.
src/​bacnet/​basic/​sys/​state_name.h Defines the configurable list maximum.
src/​bacnet/​basic/​sys/​state_name.c Enforces bounds and initializes new entries.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

@skarg
skarg merged commit 6a5ebd2 into master Sep 28, 2026
37 checks passed
@skarg
skarg deleted the bugfix/state-name-list-resize-bound branch September 28, 2026 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants