Repository navigation
BUG: C06: Feature Id not bounded by Feature Attribute Matrix in 22 filters #1866
Description
Summary
Many filters use a value from a DataArray as an index into a Feature or Ensemble DataArray, but they do not check that the value is in range. The index can be a Feature Id, a Face Label, a NeighborList entry, a Phase, or a phase index from a file. When the value is negative, or is equal to or larger than the number of tuples of the indexed DataArray (or Attribute Matrix), the filter reads or writes out of bounds. This can crash DREAM3D-NX or NXRunner, or corrupt memory silently.
As of develop 419f9db (2026-10-21), there are 23 open findings in 22 filters: 22 CRASH, 0 WRONG_DATA, 1 MINOR. All findings are candidates. They were found and re-checked by reading the code. Nobody ran the code to confirm them.
Mechanism
Preflight cannot see the values in a DataArray. Preflight can check that the Feature Ids DataArray has the same number of tuples as the Image Geometry cells, but it cannot check that the largest Feature Id is less than the number of tuples of the Feature Attribute Matrix. Execute then uses each value as an index without a range check. A Feature Attribute Matrix from a different segmentation, or one that was not updated after a crop, merge or renumber, is enough to trigger this.
Example from ComputeBoundaryElementFractions. The vectors have one entry for each tuple of the Feature Attribute Matrix, but gnum is not checked:
Lines 67 to 74 in 419f9db
| for(usize index = 0; index < count; index++) | |
| { | |
| const int32 gnum = featureIdsBuffer[index]; | |
| voxCounts[gnum]++; | |
| if(boundaryCellsBuffer[index] > 0) | |
| { | |
| surfVoxCounts[gnum]++; | |
| } |
for(usize index = 0; index < count; index++)
{
const int32 gnum = featureIdsBuffer[index];
voxCounts[gnum]++;
if(boundaryCellsBuffer[index] > 0)
{
surfVoxCounts[gnum]++;
}
}A Feature Id of -1, or a Feature Id that is equal to or larger than the number of Features, writes outside the vectors.
Fixed form, from QuickSurfaceMeshDirect (#1765). Execute finds the largest Feature Id first. Then it returns an error if any selected feature DataArray has too few tuples:
simplnx/src/Plugins/SimplnxCore/src/SimplnxCore/Filters/Algorithms/QuickSurfaceMeshDirect.cpp
Lines 856 to 865 in 419f9db
| for(const auto& featureArrayPath : m_InputValues->SelectedFeatureDataArrayPaths) | |
| { | |
| const auto& featureArray = m_DataStructure.getDataRefAs<IDataArray>(featureArrayPath); | |
| if(numFeatures >= featureArray.getNumberOfTuples()) | |
| { | |
| return MakeErrorResult( | |
| -62073, fmt::format("Feature Id {} in DataArray '{}' is outside the selected feature DataArray '{}' with {} tuples. The number of tuples must be greater than the largest Feature Id.", | |
| numFeatures, m_InputValues->FeatureIdsArrayPath.toString(), featureArrayPath.toString(), featureArray.getNumberOfTuples())); | |
| } | |
| } |
for(const auto& featureArrayPath : m_InputValues->SelectedFeatureDataArrayPaths)
{
const auto& featureArray = m_DataStructure.getDataRefAs<IDataArray>(featureArrayPath);
if(numFeatures >= featureArray.getNumberOfTuples())
{
return MakeErrorResult(
-62073, fmt::format("Feature Id {} in DataArray '{}' is outside the selected feature DataArray '{}' with {} tuples. The number of tuples must be greater than the largest Feature Id.",
numFeatures, m_InputValues->FeatureIdsArrayPath.toString(), featureArrayPath.toString(), featureArray.getNumberOfTuples()));
}
}Rule
- The range check must run in execute, because preflight cannot see the values.
- At the start of execute, call
ValidateFeatureIdsToFeatureAttributeMatrixIndexing(src/simplnx/Utilities/DataArrayUtilities.hpp). If the filter has a cancel flag, use the overload that takesconst std::atomic_bool* shouldCancel. About 26 source files already use this helper. As an alternative, check each access and return an error before the out-of-bounds access. - If the index comes from a parameter value (for example Phase Index), check it in preflight, because preflight can see parameter values.
- If preflight can compare tuple counts (for example Phases and Average Euler Angles must have the same number of tuples), add that check in preflight too.
- Remove any execute-time check that a new check makes unreachable. Do not keep duplicate checks.
How to fix
- Write the failing test first (see "How to test").
- For a 1-component
Int32Arrayof Feature Ids or Phases, call the helper at the start of execute:ValidateFeatureIdsToFeatureAttributeMatrixIndexing(dataStructure, sourceDataPath, featureIds, ignoreNegativeValues, messageHandler, &shouldCancel)sourceDataPathcan be the Attribute Matrix or one feature-level DataArray. Use the DataArray (or the Attribute Matrix) that the code indexes. SetignoreNegativeValuestotrueonly if the code skips negative values before it indexes. The helper returns -5355 for a negative value and -5351 for a value that is too large. - The helper reads only
getNumberOfTuples()values from the start of the store. It does not check all values of a 2-componentFace LabelsDataArray, and it does not accept a NeighborList or a non-int32DataArray. For these inputs, scan the minimum and maximum yourself (in chunks, with no full-size temporary array), and return an error with a code from the filter's range. - When an input has several Feature DataArrays (for example Average Quaternions and Phases), check against the smallest tuple count, or require equal tuple counts in preflight.
- For file readers (
ReadH5EbsdFilter,ReadH5EspritDataFilter,ReadChannel5DataFilter), the Ensemble Attribute Matrix must have (largest phase index + 1) tuples. Check each cell Phase value against that count. Check that each lattice constant vector has 6 values before you read it. - New error codes must use the range of that filter. Filter error codes are more negative than -10000, and each filter uses one range of 100 codes (first code -47500 gives -47500..-47599). Check that the code is unique with
git grep -nE -- "-NNNN([^0-9]|$)". Do not use\b, becausegit grep -Eignores it. Also check the open pull request branches.
How to test
- Write a unit test that fails on develop before you change the code. On develop the test crashes or reads out of bounds. Run it with AddressSanitizer if you can.
- Execute test: build a small DataStructure where one Feature Id (or Face Label, NeighborList entry, or Phase) is equal to the number of tuples of the Feature or Ensemble Attribute Matrix. Assert that execute returns the expected error code.
- Add a second case with a negative value (-1) where the code does not skip negative values.
- If the filter has Direct and Scanline algorithms, run both through the existing algorithm-selection test scope.
- Add one valid-input case to show that the new check does not reject good data. Compare the output values with an independent expected result, not with the output of the code under test.
- If you cannot reproduce a finding, write this with the reason in the pull request and on this issue. Do not force a fix.
Conventions
- Make one pull request for the whole class, with one commit for each filter.
- Start each commit message with
BUG:and add aSigned-off-byline. Do not add AI attribution. - Start each fix with a unit test that fails on develop.
- Value checks need an independent expected result (oracle), not the output of the code under test.
- Run ctest serially. Never use
-j, because the tests depend on each other. - Format with clang-format 19.1.1 and the repository
.clang-format. - Do not add sentences about input requirements to the filter documentation. The generated parameter table documents them.
- If a fix touches a filter that has a V&V report (
src/Plugins/<Plugin>/vv/<Filter>.md), update every file of its V&V package (report, deviations, provenance, test tables). V&V documents reference DREAM3D 6.5.171 only. - Rebase on develop. Do not merge develop into the branch.
Checklist
- CRASH · ComputeBoundaryElementFractionsFilter · ComputeBoundaryElementFractions.cpp:70 — A Cell Feature Id that is negative or >= the Feature Attribute Matrix tuple count writes outside the count vectors.
ComputeBoundaryElementFractionsFilter#1 - CRASH · ComputeBoundaryStrengthsFilter · ComputeBoundaryStrengths.cpp:67 — A Face Label >= the tuple count of Average Quaternions or Phases reads out of bounds; preflight does not compare the two Feature DataArrays either.
ComputeBoundaryStrengthsFilter#1 - CRASH · ComputeFeatureClusteringFilter · ComputeFeatureClustering.cpp:273 — Phase Index = 5 (or -1) with a 3-tuple Cell Ensemble Attribute Matrix writes out of bounds. This is a parameter value, so check it in preflight.
ComputeFeatureClusteringFilter#1 - CRASH · ComputeGBCDFilter · ComputeGBCD.cpp:448 — A Face Label >= the Phases tuple count reads out of bounds; Phases with more tuples than Average Euler Angles also reads the Euler cache out of bounds.
ComputeGBCDFilter#1 - CRASH · ComputeGBCDMetricBasedFilter · ComputeGBCDMetricBased.cpp:128 — A Face Label or Feature Face Label of 12 with 10 Features reads the Phases and Euler caches out of bounds in the parallel workers.
ComputeGBCDMetricBasedFilter#2 - CRASH · ComputeGBPDMetricBasedFilter · ComputeGBPDMetricBased.cpp:113 — A Face Label >= the Feature tuple count, or Average Euler Angles with fewer tuples than Phases, reads out of bounds.
ComputeGBPDMetricBasedFilter#3 - CRASH · ComputeShapesTriangleGeomFilter · ComputeShapesTriangleGeom.cpp:525 —
FindEulerCharacteristicValuesreturns (largest Face Label + 1) values, andstd::copywrites them into a smaller Euler Characteristic store (e.g. label 120, 100 tuples): out-of-bounds write.ComputeShapesTriangleGeomFilter#3 - CRASH · ComputeSlipTransmissionMetricsFilter · ComputeSlipTransmissionMetrics.cpp:68 — A Feature Neighbor List entry that is negative or >= the Feature Phases tuple count reads Average Quaternions and Phases out of bounds.
ComputeSlipTransmissionMetricsFilter#2 - CRASH · ComputeTwinBoundariesFilter · ComputeTwinBoundaries.cpp:420 — A positive Face Label >= the Feature Phases tuple count reads the phase vector out of bounds; the parallel workers do the same with Average Quaternions.
ComputeTwinBoundariesFilter#1 - CRASH · ComputeVolumeFractionsFilter · ComputeVolumeFractionsFilter.cpp:139 — A Cell Phases value of 3 (or -1) with a 3-tuple Cell Ensemble Attribute Matrix writes outside the per-ensemble counts.
ComputeVolumeFractionsFilter#1 - CRASH · CreateAMScanPathsFilter · CreateAMScanPaths.cpp:397 — An edge Slice Id or Region Id of -1 indexes the buckets out of bounds; a value of 2147483647 makes the +1 overflow.
CreateAMScanPathsFilter#3 - CRASH · FeatureFaceCurvatureFilter · FeatureFaceCurvature.cpp:92 — Only the maximum Feature Face Id is scanned; a face with Id -1 writes before the start of
faceSizes(heap corruption) and also adds a -1 group.FeatureFaceCurvatureFilter#3 - CRASH · M3CSurfaceMeshingFilter · M3CSurfaceMeshing.cpp:3107 — Feature Ids 1..10 with a 5-tuple feature DataArray to transfer:
quickSurfaceTransferreads out of bounds on the default in-core path. Add the -62073-style check that QuickSurfaceMeshDirect has.M3CSurfaceMeshingFilter#2 - CRASH · MergeTwinsFilter · MergeTwins.cpp:301 — A Cell Feature Id >= the Phases tuple count, or a Contiguous Neighbor List entry >= the number of Features, reads and writes the parent-id and feature arrays out of bounds.
MergeTwinsFilter#3 - CRASH · RandomizeFeatureIdsFilter · ClusteringUtilities.cpp:94 — Execute checks only the largest Feature Id; a Feature Id of -1 (e.g. {-1, 0, 1, 2}) indexes
randomIdsout of bounds.RandomizeFeatureIdsFilter#2 - CRASH · ReadH5EbsdFilter · ReadH5Ebsd.cpp:296 — An HKL .h5ebsd cell Phase that is negative or larger than the phase count in the header reads the crystal structure cache out of bounds.
ReadH5EbsdFilter#3 - CRASH · ReadH5EbsdFilter · ReadH5Ebsd.cpp:106 — Phase indices that are not contiguous from 1 (e.g. 1 and 3) write the Ensemble DataArrays out of bounds; a LatticeConstants vector with fewer than 6 values is read out of bounds.
ReadH5EbsdFilter#4 - CRASH · ReadH5EspritDataFilter · IEbsdOemReader.hpp:163 — Non-contiguous phase indices, or a later scan with more phases than the first scan, write the Ensemble DataArrays out of bounds; lattice constants are read without a size check.
ReadH5EspritDataFilter#2 - CRASH · RequireMinimumSizeFeaturesFilter · RequireMinimumSizeFeatures.cpp:197 — A Cell Feature Id of 10 with a 5-tuple Feature Attribute Matrix reads
activeObjectsandnewNamesout of bounds.RequireMinimumSizeFeaturesFilter#1 - CRASH · UncertainRegularGridSampleSurfaceMeshFilter · SampleSurfaceMesh.cpp:279 —
linkLochas one entry per face, but it is indexed by Feature Id; a 4-face Triangle Geometry with Face Labels {5,0} reads and writes out of bounds. Size it by the number of Features.UncertainRegularGridSampleSurfaceMeshFilter#1 - CRASH · WriteGBCDTriangleDataFilter · WriteGBCDTriangleData.cpp:95 — Only negative labels are skipped; a Face Label of 25 with a 10-tuple Average Euler Angles reads the Euler cache out of bounds.
WriteGBCDTriangleDataFilter#1 - CRASH · WriteINLFileFilter · WriteINLFile.cpp:306 — The crystal structure is read before the
phaseId > 0test; a Cell Phase of -1, or 3 with 2 Crystal Structures tuples, reads out of bounds.WriteINLFileFilter#3 - MINOR · ReadChannel5DataFilter · ReadChannel5Data.cpp:265 — A .crc Phase value larger than the .cpr phase count is written to Phases with no error. The values match the file, but later filters can index out of bounds. Return an error, as the Ctf reader does with -19602.
ReadChannel5DataFilter#4
Already fixed or in progress
- BUG: Fix wrong output and invalid input handling in 19 filters #1765 (merged, 1986127):
ComputeFeatureBoundsFilter(ComputeFeatureBoundsFilter#3),QuickSurfaceMeshFilter(QuickSurfaceMeshFilter#3),SilhouetteFilter(SilhouetteFilter#1),SurfaceNetsFilter(SurfaceNetsFilter#1). - BUG: Enforce component shapes and types that filters read #1766 (open):
ComputeNumFeaturesFilter(ComputeNumFeaturesFilter#2).
Out of scope
- Findings of this class in filters that have a V&V report are not in this issue. There are 11 such findings, in these filters:
ComputeAvgCAxesFilter,ComputeFaceIPFColoringFilter,ComputeFeatureFaceMisorientationFilter,ComputeFeatureNeighborCAxisMisalignmentsFilter,ComputeFeatureNeighborMisorientationsFilter,ComputeFeatureReferenceCAxisMisorientationsFilter,ComputeGroupingDensityFilter,ErodeDilateBadDataFilter,GroupMicroTextureRegionsFilter,ReadAngDataFilter. They are handled on a separate V&V branch. - Preflight checks that the Feature Ids DataArray has the same number of tuples as the geometry cells, or is in the geometry's Cell Attribute Matrix, are a different class. This issue is only about the values.
Activity
[-]BUG: Feature Id not bounded by Feature Attribute Matrix in 22 filters[/-][+]BUG: C06: Feature Id not bounded by Feature Attribute Matrix in 22 filters[/+]