Skip to content

perf_hooks: validate import normalization offset - #65950

Merged
nodejs-github-bot merged 1 commit into
nodejs:mainfrom
mcollina:perf-hooks-validate-import-norm-offset
Sep 12, 2026
Merged

perf_hooks: validate import normalization offset#65950
nodejs-github-bot merged 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 147 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.

@mcollina
mcollina requested review from jasnell and legendecas and removed request for jasnell September 10, 2026 15:47
@legendecas legendecas added request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. author ready PRs with CI started, the required approvals, and no outstanding review comments. labels Sep 10, 2026
@github-actions github-actions Bot removed the request-ci Add this label to start a Jenkins CI on a PR. Only starts once the PR has an approving review. label Sep 10, 2026
@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment was marked as outdated.

@nodejs-github-bot

This comment has been minimized.

@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

@panva panva added the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 12, 2026
@nodejs-github-bot
nodejs-github-bot merged commit 40b1821 into nodejs:main Sep 12, 2026
107 of 109 checks passed
@nodejs-github-bot

Copy link
Copy Markdown
Collaborator

Landed in 40b1821

@nodejs-github-bot nodejs-github-bot removed the commit-queue PRs queued for automated landing through the Commit Queue. label Sep 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

author ready PRs with CI started, the required approvals, and no outstanding review comments. 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.

5 participants