Skip to content

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

Open
aria-googler wants to merge 1 commit into
mainfrom
fix/detector-coords-indexing
Open

fix: key detector coordinates by detector ID and bounds check array a…#272
aria-googler wants to merge 1 commit into
mainfrom
fix/detector-coords-indexing

Conversation

@aria-googler

Copy link
Copy Markdown

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/...

@aria-googler
aria-googler requested a review from a team as a code owner July 26, 2026 19:19
@aria-googler
aria-googler requested review from LalehB and removed request for a team July 26, 2026 19:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant