Bruker 2dseq reader: ParaVision on-disk format conformance

[gdevenyi]

Parse the JCAMP-DX forms ParaVision writes (PV5.1 headers, PV360 RLE and enum arrays, strings with commas), fix frame-scaling cardinality, and derive slice count and direction from frame groups. Adds a synthetic PV360 GTest, no new external test data.

Defects fixed, per commit

• Parser: the fixed five-##/three-$$ header assumption desynchronized on PV5.1 files (two $$ lines — VisuVersion was silently consumed) and could not read ParaVision 360 files at all: @N*(value) run-length encoded arrays, $$ @vis= comments inside wrapped value blocks, commas inside <> strings (e.g. <Parameter maps T2 relaxation, bg: Otsu.>), enum values stored as sized arrays (( 1 ) + disk_normal_slice_order), and scalar struct values on the parameter line.
• Scaling: VisuCoreDataSlope/VisuCoreDataOffs may hold one value for all frames or one per frame; per-frame indexing read out of bounds when a single value was stored.
• Geometry: 2D datasets whose frame groups lack FG_SLICE (e.g. FG_ISA parameter maps) are single-slice; deriving the slice count from identical per-frame positions produced a zero slice spacing. The slice axis now follows the sign of the slice-position step along the orientation's third row, generalizing the previous coronal-only Y-component heuristic to oblique stacks.

Test results

• Existing itkBruker2dseq_PV5.1_FSE_INT16 / PV6.0_FLASH_* regression tests pass with unchanged baselines.
• New Bruker2dseqImageIO.ReadParaVision360Dataset GTest covers RLE arrays, wrapped strings with embedded commas, mid-value comments, broadcast scaling, frame-group reordering, and a reversed slice axis.
• Local sweep over 1636 public ParaVision datasets (PV5.1, PV6.0.1, PV7, PV360 3.4-3.7 studies from Zenodo, bruker2nifti_qa, MRIReco.jl, and Bruker's PV360 standard protocols): previously 677 readable, now 1631; the 5 remaining failures are zero-byte placeholder files, rejected with a clean exception. Of the 677 previously readable, 610 outputs are byte-identical; the 67 that changed are slice-axis sign corrections on oblique/coronal 2D stacks, verified against the stored VisuCorePosition progression.

AI assistance

• Tool: Claude Code
• Role: implemented the parser rewrite and geometry fixes against the Bruker ParaVision file-format specification (derived from Bruker's D01/D12 File Formats manuals and ParaVision headers), and ran the dataset sweep above.
• All code was reviewed, built, and tested locally before committing.

[gdevenyi]

This work is built on https://github.com/gdevenyi/brkraw-legacy/blob/main/FILE_FORMAT.md which was constructed using an extensive AI deep dive into publicly available Bruker datasets, the Bruker Paravision manuals over multiple versions.

[dzenanz]

@greptileai review this.

[greptile-apps]

Greptile Summary

This update broadens ParaVision JCAMP-DX parsing, adds ParaVision 360 coverage, and adjusts Bruker metadata handling. A focused reproduction confirmed that malformed RLE metadata can make image-information reading allocate until memory is exhausted. The affected expansion code should bound both repetition counts and total decoded output before this change is merged.

Confidence Score: 3/5

Not safe to merge until Bruker RLE expansion rejects oversized metadata and enforces a checked decoded-size limit.

The vulnerable behavior was reproduced with the inspected expansion implementation under a controlled memory limit: normal RLE completed, while a maximum signed-integer repetition count exhausted the available address space. Source inspection also established that the parser is reached while image information is read.

Files Needing Attention: Modules/IO/Bruker/src/itkBruker2dseqImageIO.cxx needs bounded RLE decoding; the Bruker reader tests should add oversized-RLE rejection coverage.

Security Review

Bruker parameter files are treated as attacker-controlled input during image-information reading. A compact record such as @2147483647*(2) is accepted as a valid RLE value and expanded without a count or output-size limit, allowing a malicious dataset to consume process memory and CPU until it terminates or becomes unavailable.

T-Rex Logs

What T-Rex did

• T-Rex produced a proof for the posted P1 finding and validated it by examining the focused Bruker RLE resource-limit harness and related ExpandRLE behavior, including normal and malicious expansions and the header-parsing path.
• A second P1 finding proof was produced.
• The general-contract-validation-proof shows normal input expands to 24 bytes while malicious input exhausts the controlled 64 MiB address space, causing std::bad_alloc due to an unbounded loop in Bruker image IO, with the relevant code region and header-parsing flow identified.

_{Ran code and verified through T-Rex}

Comments Outside Diff (1)

1. General comment

   Bruker RLE count permits memory-exhaustion denial of service

   • Bug
     • A crafted visu_pars record such as ##$VisuCoreDataSlope=( 1 ) @2147483647*(2) is expanded during image-information reading. ExpandRLE appends the value and a space once per attacker-controlled count, attempting to construct roughly 4 GiB of output for this example before subsequent metadata parsing.
   • Cause
     • std::stoi accepts the maximum positive int count and the subsequent for (int c = 0; c < count; ++c) has neither a count cap nor a checked cap on the output size.
   • Fix
     • Reject RLE counts that exceed a documented safe limit and, before each expansion, use checked arithmetic to enforce a bounded total expanded size. Return a parsing exception for malformed or oversized records.

   _{Ran code and verified through T-Rex}

_{Reviews (1): Last reviewed commit: "STYLE: Avoid brace-value initializers fl..." | Re-trigger Greptile}

[greptile-apps]

Unbounded RLE expansion exhausts resources

A crafted Bruker parameter record such as @2147483647*(2) reaches this loop while ReadImageInformation() parses visu_pars. The file-controlled repetition count is accepted by std::stoi and drives one append per repetition with no count or expanded-output-size limit, so parsing attempts to allocate roughly 4 GiB for this small input and can terminate the process or consume excessive CPU. Reject oversized counts and enforce checked bounds on the total expanded output before expanding RLE values.

Artifacts

Focused Bruker RLE resource-limit harness source

• This authored C++ harness copies the inspected `ExpandRLE` implementation unchanged and invokes it with normal and malicious RLE values under a controlled address-space limit, showing the vulnerable loop can be exercised safely.

Focused harness build log

• The recorded g++ command compiled the focused RLE harness successfully with exit code 0, showing the reproduction executable was built.

Normal Bruker RLE expansion output

• The recorded normal-input run expanded `@12*(2)` to 24 bytes and exited successfully, establishing the baseline behavior.

Malicious Bruker RLE expansion under memory limit

• The recorded resource-limited malicious-input run processed `@2147483647*(2)` until `std::bad_alloc`, demonstrating attempted unbounded allocation from the file-controlled count.

Inspected ExpandRLE source

• This source capture records the exact inspected implementation, including the unbounded count-controlled append loop, tying the harness to the reviewed file.

Bruker header parsing call path

• This source capture shows `ParseJCAMPDXRecord` calls `ExpandRLE` and `ReadImageInformation` reads `visu_pars`, establishing that the issue occurs while reading image information.

ITK build-environment check

• This recorded environment check found no CMake cache or ITK configuration, documenting why a fully linked ITK reader execution was unavailable.

_{Ran code and verified through T-Rex}

[gdevenyi]

Confirmed. Fixed in bc6224d: ExpandRLE now rejects repetition counts longer than nine digits and throws once the expanded record would exceed 64 MiB, so the crafted @2147483647*(2) case fails with a clean itk::ExceptionObject instead of attempting the allocation. Covered by the new Bruker2dseqImageIO.RejectOversizedRLEExpansion GTest.