Skip to content

[tree] Fix TBasket out of bounds write of fEntryOffset#22797

Merged
silverweed merged 1 commit into
root-project:masterfrom
silverweed:tbasket_entryoffset2
Jul 14, 2026
Merged

[tree] Fix TBasket out of bounds write of fEntryOffset#22797
silverweed merged 1 commit into
root-project:masterfrom
silverweed:tbasket_entryoffset2

Conversation

@silverweed

Copy link
Copy Markdown
Contributor

Following 15d2d71, fEntryOffset is (conditionally) allocated with a capacity of fNevBuf, i.e. the number of elements that are read into it.
However, one code path in WriteBuffer() writes fEntryOffset up to one past the fNevBuf-th element, causing an OOB write.

This was (usually) not happening prior to the aforementioned commit because fEntryOffset was allocated with a default capacity of 1000 (even though all elements past the fNevBuf-th were never initialized).

To fix the OOB write, we now allocate one extra element when reading fEntryOffset from disk so that the WriteBuffer write is not OOB anymore.

Bug found by asan.

Following 15d2d71, fEntryOffset is
(conditionally) allocated with a capacity of fNevBuf, i.e. the number of
elements that are read into it.
However, one code path in WriteBuffer() writes fEntryOffset up to one
past the fNevBuf-th element, causing an OOB write.

This was (usually) not happening prior to the aforementioned commit
because fEntryOffset was allocated with a default capacity of 1000
(even though all elements past the fNevBuf-th were never initialized).

To fix the OOB write, we now allocate one extra element when reading
fEntryOffset from disk so that the WriteBuffer write is not OOB anymore.
@silverweed silverweed requested a review from hageboeck July 13, 2026 11:59
@silverweed silverweed self-assigned this Jul 13, 2026
@dpiparo

dpiparo commented Jul 13, 2026

Copy link
Copy Markdown
Member

thanks for these changes! Did the bug percolate to a released version? In that case I would suggest to open an issue, fix it with this PR and backport it. Otherwise, we can consider this a transient state in the main branch and be done with it...

@github-actions

github-actions Bot commented Jul 13, 2026

Copy link
Copy Markdown

Test Results

    23 files      23 suites   3d 17h 33m 6s ⏱️
 3 874 tests  3 866 ✅   0 💤 8 ❌
79 714 runs  79 597 ✅ 109 💤 8 ❌

For more details on these failures, see this check.

Results for commit 78b0bc1.

♻️ This comment has been updated with latest results.

@silverweed

Copy link
Copy Markdown
Contributor Author

thanks for these changes! Did the bug percolate to a released version? In that case I would suggest to open an issue, fix it with this PR and backport it. Otherwise, we can consider this a transient state in the main branch and be done with it...

The original PR was not backported but is scheduled to release with 6.40.04, so we should probably backport both that PR and this PR to 6.40

@hageboeck hageboeck left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@silverweed silverweed merged commit 45efd3c into root-project:master Jul 14, 2026
62 of 64 checks passed
@silverweed silverweed deleted the tbasket_entryoffset2 branch July 14, 2026 13:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants