Skip to content

perf_hooks: validate import normalization offset - #65950

Open
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:perf-hooks-validate-import-norm-offset
Open

perf_hooks: validate import normalization offset#65950
mcollina wants to merge 1 commit into
nodejs:mainfrom
mcollina:perf-hooks-validate-import-norm-offset

Conversation

@mcollina

Copy link
Copy Markdown
Member

perf_hooks: validate import normalization offset

The CBOR importer accepts a normalizing_index_offset outside the range
of the allocated counts array. normalize_index() in hdr_histogram.c
applies at most one wrap adjustment of +/-counts_len, so an arbitrary
offset can remain out of bounds and a later record() performs an
out-of-bounds native write in counts_inc_normalised().

Reject offsets not representable as int32_t when reading the CBOR
field, and reject offsets outside [0, counts_len) after the histogram
is reconstructed. Adds regression tests for the boundary at
counts_len, offsets beyond it, and non-int32_t values.

The CBOR importer accepts a normalizing_index_offset outside the range
of the allocated counts array. normalize_index() in hdr_histogram.c
applies at most one wrap adjustment of +/-counts_len, so an arbitrary
offset can remain out of bounds and a later record() performs an
out-of-bounds native write in counts_inc_normalised().

Reject offsets that are not representable as int32_t when reading the
CBOR field, and reject offsets outside [0, counts_len) after the
histogram is reconstructed. Add regression tests covering the boundary
at counts_len, offsets beyond it, and values that cannot be represented
as int32_t.

Signed-off-by: Matteo Collina <hello@matteocollina.com>
Assisted-by: pi
@nodejs-github-bot nodejs-github-bot added c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run. labels Sep 10, 2026
@codecov

codecov Bot commented Sep 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 66.66667% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 90.17%. Comparing base (6f41e41) to head (527f21f).
⚠️ Report is 104 commits behind head on main.

Files with missing lines Patch % Lines
src/histogram.cc 66.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #65950      +/-   ##
==========================================
- Coverage   90.17%   90.17%   -0.01%     
==========================================
  Files         770      771       +1     
  Lines      264483   265488    +1005     
  Branches    50262    50472     +210     
==========================================
+ Hits       238491   239396     +905     
- Misses      16981    17044      +63     
- Partials     9011     9048      +37     
Files with missing lines Coverage Δ
src/histogram.cc 84.80% <66.66%> (-0.04%) ⬇️

... and 91 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c++ Issues and PRs that require attention from people who are familiar with C++. needs-ci PRs that need a full CI run.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants