bridgecrewio / checkov

Prevent cloud misconfigurations and find vulnerabilities during build-time in infrastructure as code, container images and open source packages with Checkov by Bridgecrew.
https://www.checkov.io/
Apache License 2.0
6.71k stars 1.07k forks source link

BaseModuleCheck: locals enrichment fails when two keys share the same prefix #6381

Open nomeelnoj opened 1 month ago

nomeelnoj commented 1 month ago

Checkov version: 3.2.116

Describe the Issue

When running a custom module check against specific types of code, the enrichment from locals fails and the conf object is populated with the literal string ${local.local_name}, rather than the actual value from the local.

This makes it particularly difficult to create custom module checks as depending on arbitrary values of keys, they may or may not be correctly replaced by the enricher, allowing code that normally would fail a check to pass.

The issue occurs when both of the following statements are true:

  1. The values being passed into the module come from locals. They can be the same local or different locals, as long as they are not directly coded in the module configuration
  2. The keys inside the object share a prefix. If foo exists alongside foobar, foo will be interpreted as the literal, while foobar will be properly enriched. This occurs with every prefix I have tried. No matter how many keys you have that share a prefix, only the longest key will be enriched properly.

Example

Here is a simplified example that shows the issue. I have found it to be reproducible in a variety of scenarios.

from checkov.common.models.enums import CheckResult, CheckCategories
from checkov.common.bridgecrew.severities import Severities
from checkov.terraform.checks.module.base_module_check import BaseModuleCheck

class Simple(BaseModuleCheck):
    def __init__(self):
        name = (
            "Test the prefix issue."
        )
        id = "CKV_FOO_2"
        categories = [CheckCategories.IAM]
        super().__init__(name=name, id=id, categories=categories)
        self.severity = Severities["HIGH"]

    def scan_module_conf(self, conf):
        for key, value in conf["permission_sets"][0].items():
            print(f"Key: {key}, Value: {value}")

        return CheckResult.PASSED

check = Simple()

An example Terraform configuration that will show the issue based on the above check.

locals {
  test = {
    sid = "foo"
  }
}

module "simple" {
  source = "../../modules/foo"
  permission_sets = {
    foo    = local.test # Not enriched, evaluates to ${local.test}
    foobar = local.test # Enriched correctly, evaluates to 
  }
}

Expected Output

Key: foo, Value: {'sid': 'foo'}
Key: foobar, Value: {'sid': 'foo'}

Actual Output

Key: foo, Value: ${local.test}
Key: foobar, Value: {'sid': 'foo'}

Additional Information

The above check shows the issue pretty clearly. It does not matter what the nested keys are, as long as at least one is the prefix of another, the shorter keys will be interpreted literally, while the longest key will be enriched correctly.

If you have even more keys that share a prefix, only the longest one shows the correct output, all keys that are substring prefixes are incorrectly enriched. Example:

permission_sets = {
  f      = local.test # Produces ${local.test}
  fo     = local.test # Produces ${local.test}
  foo    = local.test # Produces ${local.test}
  foob   = local.test # Produces ${local.test}
  fooba  = local.test # Produces ${local.test}
  foobar = local.test # Properly enriched as {'sid': 'foo"}
}

Other test cases have shown to exhibit the same incorrect behavior, meaning it does not matter what the prefix is, as long as the key is the prefix of another key, it will fail the enrichment. The local values can be the same or different (passing a different local into each key, or the same), it does not matter.

If you remove the longest key, the next longest key will be enriched. Alternatively, if you change the keys as shown below, they are enriched properly.

# All values below are properly enriched
permission_sets = {
  af      = local.test
  bfo     = local.test
  cfoo    = local.test
  dfoob   = local.test
  efooba  = local.test
  foobar  = local.test
}