Skip to content

fix(arm): don't raise IndexError on malformed resourceId()/reference() calls - #7635

Open
ankit090701 wants to merge 1 commit into
bridgecrewio:mainfrom
ankit090701:fix/arm-resourceid-indexerror-7633
Open

fix(arm): don't raise IndexError on malformed resourceId()/reference() calls#7635
ankit090701 wants to merge 1 commit into
bridgecrewio:mainfrom
ankit090701:fix/arm-resourceid-indexerror-7633

Conversation

@ankit090701

Copy link
Copy Markdown

Description

Reported as a pipeline crash (IndexError: list index out of range, re-raised from checkov/common/parallelizer/parallel_runner.py after occurring in a worker process) that only happens when the arm framework is included in a scan.

Root cause

Two functions in checkov/arm/utils.py, both used while building the ARM dependency graph (checkov/arm/graph_builder/local_graph.py), index into a str.split(...) result without checking there are enough parts:

  1. extract_resource_name_from_resource_id_func - called from _create_explicit_edge for any dependsOn entry containing the substring "resourceId(", regardless of whether the call actually has the assumed resourceId(type, name, ...) shape:

    return clean_string(resource_id.split(',')[1].split(')')[0])

    A single-argument (or otherwise malformed) resourceId() call has no comma to split on, so [1] raises IndexError.

  2. extract_resource_name_from_reference_func has a mismatched guard: it checks 'resourceId' in resource_name (no trailing paren) but then unconditionally does resource_name.split('resourceId(', 1)[1] (with the paren). Any resource/variable name that merely contains "resourceId" as a substring without being immediately followed by "(" - e.g. resourceIdentifier - passes the guard but fails the split, raising IndexError instead of falling through to the plain-reference handling every other shape uses.

Fix

  • Add a length check before indexing in extract_resource_name_from_resource_id_func; fall back to the raw (cleaned) string if the split doesn't have the expected shape, rather than crashing.
  • Tighten extract_resource_name_from_reference_func's guard to check for 'resourceId(' (matching what the subsequent split actually looks for), so the two can never disagree.

Both changes are minimal and preserve existing behavior for every well-formed input (verified against the existing test cases).

Testing

  • Added test_extract_resource_name_from_resource_id_func (a new test - the function had no dedicated tests before) covering both the documented multi-argument shape and a three-argument nested-resource shape.
  • Added test_extract_resource_name_from_resource_id_func_single_arg, reproducing the crash class from this issue with a single-argument resourceId() call.
  • Added test_extract_resource_name_from_reference_func_resource_id_substring_without_call, reproducing the second crash with a resource name (resourceIdentifier) that contains "resourceId" as a substring without a call.
  • Verified by reverting just the checkov/arm/utils.py change: both new crash-reproducing tests fail with IndexError: list index out of range at the exact lines described above.
  • Ran the full tests/arm/ suite (pytest tests/arm/): 211 passed. The only remaining issue (tests/arm/graph_builder/checks/test_yaml_policies.py) fails to even collect in my minimal Docker environment due to a missing graph_framework test-parametrization setup unrelated to this change (nothing in this PR touches graph-framework/DB-connector selection).

Fixes #7633

…) calls

extract_resource_name_from_resource_id_func indexed resource_id.split(',')[1]
unconditionally, but its only caller (_create_explicit_edge, used while
building the ARM dependency graph) triggers it for any dependsOn entry that
merely contains the substring "resourceId(" - it does not verify the call
actually has the expected "resourceId(type, name, ...)" shape. A
single-argument or otherwise malformed resourceId() call therefore has no
comma to split on, and IndexError propagates out of graph building,
crashing the whole scan (checkov/common/parallelizer/parallel_runner.py
re-raises worker exceptions in the main process).

extract_resource_name_from_reference_func has the same class of bug: it
checks `'resourceId' in resource_name` (no trailing paren) but then
unconditionally indexes `resource_name.split('resourceId(', 1)[1]` (with
the paren). Any resource/variable name that merely contains "resourceId"
as a substring without being immediately followed by "(" - e.g.
resourceIdentifier - passes the check but fails the split, so this also
raises IndexError instead of falling through to the plain-reference
handling used by every other shape.

Add a length check before indexing in the first function, and tighten the
second function's guard to require the same substring the split expects.

Fixes bridgecrewio#7633
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.

ARM Framework issue in Checkov (Azure Devops)

2 participants