fix: key detector coordinates by detector ID and bounds check array a…

[aria-googler]

Summary

Fixes an unchecked heap out-of-bounds write vulnerability and coordinate mismatch in get_detector_coords and SimplexDecoder when parsing Detector Error Model (.dem) files containing duplicate, sparse, or out-of-order detector coordinate declarations.

---

Root Cause & Problem

1. Sequential Appending in get_detector_coords:
   Previously, get_detector_coords in src/utils.cc appended coordinate vectors sequentially as DEM_DETECTOR instructions were encountered, rather than mapping coordinates to target detector IDs (target.val()).

   • If a DEM file declared duplicate coordinate instructions for the same detector (e.g., detector(0,0,1) D0 and detector(0,0,2) D0), detector_coords.size() became greater than dem.count_detectors().
   • If a DEM file declared sparse or out-of-order detectors (e.g., detector(1,2,3) D5), detector_coords[0] held the coordinates for $D_5$ instead of index 5.

2. Unchecked Heap Writes:
   In SimplexDecoder (src/simplex.cc) and build_det_orders_coordinate (src/utils.cc), iteration loops ran up to detector_coords.size() and wrote into buffers (detector_t_coords and inner_products) sized to dem.count_detectors(). When detector_coords.size() > dem.count_detectors(), an out-of-bounds heap write occurred.

---

Key Changes

• Detector ID Keying (src/utils.cc):

  • Updated get_detector_coords to allocate a detector_coords vector of size dem.count_detectors() and store coordinates at detector_coords[det_id] using target.val().
  • Preserved backward compatibility by returning an empty vector {} when no DEM_DETECTOR instructions are present in the DEM.

• Defensive Bounds Checking (src/simplex.cc & src/utils.cc):

  • Added std::min(...) bounds checking in SimplexDecoder::SimplexDecoder and build_det_orders_coordinate to guarantee loop indices never exceed allocation sizes.

• Unit Tests (src/tesseract.test.cc):

  • Added TEST(utils, DuplicateDetectorCoords) and TEST(simplex, DuplicateDetectorCoords) to verify safe coordinate mapping and non-crashing behavior on DEM files with duplicate detector coordinate declarations.

---

Verification & Testing

• C++ Formatting: Formatted all modified files with clang-format -i matching repository CI guidelines.
• Bazel Test Suite: All 14 test targets pass with zero failures:

  bazel test //src/...

[LalehB]

This still assumes D0 defines the coordinate dimensionality. If D0 has no coordinates but a later detector does, the check above returns identity orderings. If D0 has fewer dimensions, the extra dimensions on later detectors are ignored because orientation_vector is sized from detector_coords[0].
Could we determine the maximum coordinate length across all detectors, and only fall back to identity when every coordinate vector is empty?

[LalehB]

Can we add a regression test where D0 has no coordinates but D1 does? That’s the case that exposes the coordinate-ordering issue