Skip to content

Fix stack buffer overflow in Schedule_Weekly_Schedule_Set#1222

Merged
skarg merged 2 commits intobacnet-stack:masterfrom
xiang33:fix/schedule-overflow-clean-v2
Feb 7, 2026
Merged

Fix stack buffer overflow in Schedule_Weekly_Schedule_Set#1222
skarg merged 2 commits intobacnet-stack:masterfrom
xiang33:fix/schedule-overflow-clean-v2

Conversation

@xiang33
Copy link
Contributor

@xiang33 xiang33 commented Feb 6, 2026

Summary

Fix stack buffer overflow in Schedule_Weekly_Schedule_Set() function.

Problem

The memcpy in Schedule_Weekly_Schedule_Set() was using sizeof(BACNET_WEEKLY_SCHEDULE) instead of sizeof(BACNET_DAILY_SCHEDULE). This caused it to read 6784 bytes from a 968-byte source
buffer, leading to:

Stack buffer overflow detected by AddressSanitizer
Segmentation fault in test_schedule unit test
Test failure: 99% tests passed, 1 test failed
Root Cause

// Before (incorrect):
memcpy(&pObject->Weekly_Schedule[array_index], value,
       sizeof(BACNET_WEEKLY_SCHEDULE));  // 6784 bytes

// After (correct):
memcpy(&pObject->Weekly_Schedule[array_index], value,
       sizeof(BACNET_DAILY_SCHEDULE));  // 968 bytes

Fix

Changed the memcpy size parameter from sizeof(BACNET_WEEKLY_SCHEDULE) to sizeof(BACNET_DAILY_SCHEDULE) since we are copying a single daily schedule element to the array at the specified
index.

Test Results

All 262 tests now pass (100%)
No AddressSanitizer errors
test_schedule test now passes
Checklist

Code compiles cleanly
All tests pass
Tested with AddressSanitizer - no memory errors
Follows project coding style

The memcpy was using sizeof(BACNET_WEEKLY_SCHEDULE) instead of
sizeof(BACNET_DAILY_SCHEDULE), causing it to read 6784 bytes from
a 968-byte source buffer, leading to stack buffer overflow and
segmentation fault in the test_schedule unit test.

This fixes the test_schedule test failure detected by ASAN:
ERROR: AddressSanitizer: stack-buffer-overflow on address ...
READ of size 6784 at 0x... partially underflows this variable

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Fixes a memory-safety bug in the Schedule object implementation by correcting the memcpy() size used when setting a single daily schedule entry within Schedule_Weekly_Schedule_Set().

Changes:

  • Correct memcpy() length from sizeof(BACNET_WEEKLY_SCHEDULE) to sizeof(BACNET_DAILY_SCHEDULE) when copying one array element.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@skarg skarg merged commit 741748f into bacnet-stack:master Feb 7, 2026
36 checks passed
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.

3 participants