Skip to content

NDPluginAttrPlot: fix <= off-by-one that grows the attribute list past its buffers#595

Merged
MarkRivers merged 1 commit into
areaDetector:masterfrom
physwkim:fix/attrplot-off-by-one
Jul 16, 2026
Merged

NDPluginAttrPlot: fix <= off-by-one that grows the attribute list past its buffers#595
MarkRivers merged 1 commit into
areaDetector:masterfrom
physwkim:fix/attrplot-off-by-one

Conversation

@physwkim

Copy link
Copy Markdown
Contributor

rebuild_attributes() guards discovery with attributes_.size() <= n_attributes_; testing size before the append lets the list reach n_attributes_ + 1. data_ holds exactly n_attributes_ circular buffers, so push_data()'s for (i<length) data_[i].push_back(...) reaches data_[n_attributes_] — a heap out-of-bounds operator[] on the first frame whenever an NDArray carries more numeric attributes than configured. Change <= to <. Proven with an AddressSanitizer driver: 5 numeric attrs against n_attributes_=4 overflows before, completes clean after.

…uffers

rebuild_attributes() discovered attributes with the loop guard

    attr != NULL && attributes_.size() <= n_attributes_

The size is tested before the push_back inside the body, so when
attributes_.size() == n_attributes_ the condition n <= n is still true,
the body runs, and one more name is appended -- leaving
attributes_.size() == n_attributes_ + 1.

data_ is constructed with exactly n_attributes_ circular buffers. On the
next frame push_data() takes length = attributes_.size() and loops
for (i < length) data_[i].push_back(...), so data_[n_attributes_] is an
out-of-bounds operator[] on the buffer vector -- a heap out-of-bounds
write, reachable on the first frame whenever an NDArray carries more
numeric attributes than the configured n_attributes.

Use < so the list is capped at exactly n_attributes_ entries, matching
the buffer count.

Verified with an AddressSanitizer proof driver: with <= the list reaches
5 entries against 4 buffers and data_[4].push_back triggers a
heap-buffer-overflow; with < it stops at 4 and completes cleanly.
@MarkRivers
MarkRivers merged commit 8dca719 into areaDetector:master Jul 16, 2026
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