From 9d200ca22eda3b21f6978f8589b8e1df2a8fc3ab Mon Sep 17 00:00:00 2001 From: evilgensec Date: Wed, 17 Jun 2026 07:48:11 +0545 Subject: [PATCH] Bound 'B' aux array element count in ParseAuxFields The BAM aux 'B' (array) branch in ParseAuxFields read the element count (n_elements) directly from the record bytes and then iterated that many elements, dereferencing the buffer on each iteration, without checking the count against the bytes remaining in the record. Every scalar aux branch in the same function already bounds-checks (`end - s < size`); the 'B' array branch did not. A BAM whose 'B' array declares more elements than the record contains caused a heap out-of-bounds read past the htslib record buffer when parse_aux_fields is enabled (e.g. make_examples --parse_sam_aux_fields, which DeepTrio enables by default). Reject an element count whose total byte size exceeds the remaining record, computed in 64-bit since n_elements is read as a uint32 into an int. --- third_party/nucleus/io/sam_reader.cc | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/third_party/nucleus/io/sam_reader.cc b/third_party/nucleus/io/sam_reader.cc index 04781d78..7d2cbfe0 100644 --- a/third_party/nucleus/io/sam_reader.cc +++ b/third_party/nucleus/io/sam_reader.cc @@ -372,6 +372,15 @@ ::nucleus::Status ParseAuxFields(const bam1_t* b, // We need to skip 4 bytes for n_elements int that occurs before the // array. s += 4; + // Reject an element count that would walk past the end of the + // record. Every scalar aux branch above already bounds-checks; the + // 'B' array branch must too. Compute in 64-bit: n_elements is read as + // a uint32 into an int, so a value >= 2^31 is itself negative. + if (n_elements < 0 || + static_cast(n_elements) * element_size > (end - s)) { + return ::nucleus::DataLoss("B-array length exceeds record for tag " + + tag); + } if (sub_type == 'c') { std::vector all_values; for (int i = 0; i < n_elements; i++) {