Skip to content

Commit 5e2fbfe

Browse files
mcollinaaduh95
authored andcommitted
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 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 PR-URL: #65950 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent 3325ded commit 5e2fbfe

2 files changed

Lines changed: 65 additions & 0 deletions

File tree

src/histogram.cc

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -958,6 +958,10 @@ std::shared_ptr<Histogram> Histogram::Import(const uint8_t* data, size_t len) {
958958
case kKeyNormOffset: {
959959
uint64_t v;
960960
if (!CborReadUint(p, end, &v)) return nullptr;
961+
// Reject values that cannot be represented as int32_t; the
962+
// static_cast below would wrap and produce an arbitrary offset.
963+
if (v > static_cast<uint64_t>(std::numeric_limits<int32_t>::max()))
964+
return nullptr;
961965
norm_offset = static_cast<int32_t>(v);
962966
break;
963967
}
@@ -1049,6 +1053,13 @@ std::shared_ptr<Histogram> Histogram::Import(const uint8_t* data, size_t len) {
10491053
// Validate counts_len matches what the options produce.
10501054
if (histogram->histogram_->counts_len != counts_len) return nullptr;
10511055

1056+
// The normalization offset must index into the allocated counts array.
1057+
// normalize_index() in hdr_histogram.c applies at most one wrap
1058+
// adjustment of +/-counts_len, so any offset outside [0, counts_len)
1059+
// can leave normalized_index out of bounds and a later record() would
1060+
// perform an out-of-bounds write in counts_inc_normalised().
1061+
if (norm_offset < 0 || norm_offset >= counts_len) return nullptr;
1062+
10521063
// Restore counts directly.
10531064
for (const auto& [idx, cnt] : sparse_counts) {
10541065
if (idx < 0 || idx >= counts_len) return nullptr;

test/parallel/test-perf-hooks-histogram-stats.js

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -653,6 +653,60 @@ const { createHistogram, importHistogram } = require('perf_hooks');
653653
0x09, 0x18, 0x40, // 9 (countsLen) = 64
654654
0x0a, 0x82, 0x18, 0x64, 0x01, // 10 (counts) = [100, 1]
655655
])), { code: 'ERR_INVALID_ARG_VALUE' });
656+
657+
// --- Normalization offset validation ---
658+
659+
// lowest=1, highest=100, figures=1 produces counts_len=64. An offset
660+
// at or beyond counts_len leaves normalize_index() out of bounds after
661+
// its single wrap adjustment and would corrupt memory on a later
662+
// record(), so it must be rejected.
663+
assert.throws(() => importHistogram(new Uint8Array([
664+
0xa4, // map(4)
665+
0x02, 0x18, 0x64, // 2 (highest) = 100
666+
0x03, 0x01, // 3 (figures) = 1
667+
0x09, 0x18, 0x40, // 9 (countsLen) = 64
668+
0x07, 0x18, 0x40, // 7 (normOffset) = 64
669+
])), { code: 'ERR_INVALID_ARG_VALUE' });
670+
// Offset just beyond the accepted range.
671+
assert.throws(() => importHistogram(new Uint8Array([
672+
0xa4, // map(4)
673+
0x02, 0x18, 0x64, // 2 (highest) = 100
674+
0x03, 0x01, // 3 (figures) = 1
675+
0x09, 0x18, 0x40, // 9 (countsLen) = 64
676+
0x07, 0x18, 0x65, // 7 (normOffset) = 101
677+
])), { code: 'ERR_INVALID_ARG_VALUE' });
678+
// Offset that cannot be represented as int32_t (2**32). Without the
679+
// representability check the static_cast would wrap to 0 and accept it.
680+
assert.throws(() => importHistogram(new Uint8Array([
681+
0xa4, // map(4)
682+
0x02, 0x18, 0x64, // 2 (highest) = 100
683+
0x03, 0x01, // 3 (figures) = 1
684+
0x09, 0x18, 0x40, // 9 (countsLen) = 64
685+
0x07, 0x1b, 0x00, 0x00, 0x00, 0x01, // 7 (normOffset) = 2**32
686+
0x00, 0x00, 0x00, 0x00,
687+
])), { code: 'ERR_INVALID_ARG_VALUE' });
688+
689+
// Valid offsets within [0, counts_len) are still accepted: 0 and the
690+
// highest valid offset (counts_len - 1).
691+
const offsetZero = importHistogram(new Uint8Array([
692+
0xa4, // map(4)
693+
0x02, 0x18, 0x64, // 2 (highest) = 100
694+
0x03, 0x01, // 3 (figures) = 1
695+
0x09, 0x18, 0x40, // 9 (countsLen) = 64
696+
0x07, 0x00, // 7 (normOffset) = 0
697+
]));
698+
assert.strictEqual(offsetZero.count, 0);
699+
const offsetMax = importHistogram(new Uint8Array([
700+
0xa4, // map(4)
701+
0x02, 0x18, 0x64, // 2 (highest) = 100
702+
0x03, 0x01, // 3 (figures) = 1
703+
0x09, 0x18, 0x40, // 9 (countsLen) = 64
704+
0x07, 0x18, 0x3f, // 7 (normOffset) = 63
705+
]));
706+
assert.strictEqual(offsetMax.count, 0);
707+
// The imported histogram with the max valid offset is recordable.
708+
offsetMax.record(1);
709+
assert.strictEqual(offsetMax.count, 1);
656710
}
657711

658712
// ---------------------------------------------------------------------------

0 commit comments

Comments
 (0)