Fix an issue with endless loops in Fn::Sub

aws-cloudformation · Jul 14, 2024 · 5fc5cf1 · 5fc5cf1
1 parent ee6271f
commit 5fc5cf1
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 20 deletions.
diff --git a/src/cfnlint/rules/functions/Sub.py b/src/cfnlint/rules/functions/Sub.py
@@ -81,7 +81,7 @@ def _clean_error(
         return err
 
     def _validate_string(
-        self, validator: Validator, key: str, instance: Any
+        self, validator: Validator, key: str, instance: Any, sub_values: dict[str, Any]
     ) -> ValidationResult:
         params = re.findall(REGEX_SUB_PARAMETERS, instance)
         validator = validator.evolve(
@@ -102,26 +102,36 @@ def _validate_string(
                 for err in validator.descend(
                     instance={"Fn::GetAtt": param},
                     schema={"type": ["string"]},
-                    path=key,
                 ):
                     yield self._clean_error(err, {"Fn::GetAtt": param}, param)
             else:
-                for err in validator.descend(
-                    instance={"Ref": param},
-                    schema={"type": ["string"]},
-                    path=key,
-                ):
-                    yield self._clean_error(err, {"Ref": param}, param)
+                if param in sub_values:
+                    for err in validator.descend(
+                        instance=sub_values[param],
+                        schema={"type": ["string"]},
+                        path=param,
+                    ):
+                        yield self._clean_error(err, sub_values[param], param)
+                else:
+                    for err in validator.descend(
+                        instance={"Ref": param},
+                        schema={"type": ["string"]},
+                    ):
+                        yield self._clean_error(err, {"Ref": param}, param)
 
     def fn_sub(
         self, validator: Validator, s: Any, instance: Any, schema: Any
     ) -> ValidationResult:
         validator = validator.evolve(
-            context=validator.context.evolve(strict_types=True)
+            context=validator.context.evolve(strict_types=True),
         )
-        yield from super().validate(validator, s, instance, schema)
+        errs = list(super().validate(validator, s, instance, schema))
+        if errs:
+            yield from iter(errs)
+            return
 
         key, value = self.key_value(instance)
+        sub_values = {}
         if validator.is_type(value, "array"):
             if len(value) != 2:
                 return
@@ -133,16 +143,11 @@ def fn_sub(
                 fn_k, fn_v = is_function(sub_v)
                 if fn_k == "Ref" and fn_v == sub_k:
                     del sub_values[sub_k]
-            validator_string = validator.evolve(
-                context=validator.context.evolve(ref_values=sub_values)
-            )
             value = value[0]
-        elif validator.is_type(value, "string"):
-            validator_string = validator
-        else:
+        elif not validator.is_type(value, "string"):
             return
 
-        errs = list(self._validate_string(validator_string, key, value))
+        errs = list(self._validate_string(validator, key, value, sub_values))
         if errs:
             yield from iter(errs)
             return

diff --git a/test/unit/rules/functions/test_sub.py b/test/unit/rules/functions/test_sub.py
@@ -10,6 +10,7 @@
 from cfnlint.context import create_context_for_template
 from cfnlint.jsonschema import CfnTemplateValidator, ValidationError
 from cfnlint.rules.functions.GetAtt import GetAtt
+from cfnlint.rules.functions.GetAz import GetAz
 from cfnlint.rules.functions.Ref import Ref
 from cfnlint.rules.functions.Sub import Sub
 from cfnlint.template import Template
@@ -199,10 +200,26 @@ def context(cfn):
                     ),
                     validator="fn_sub",
                 ),
+            ],
+        ),
+        (
+            "Invalid Fn::Sub with a bad object type from fn",
+            {
+                "Fn::Sub": [
+                    "${foo}",
+                    {
+                        "foo": {"Fn::GetAZs": ""},
+                    },
+                ]
+            },
+            {"type": "string"},
+            [
                 ValidationError(
-                    ("'foo' is not of type 'string' " "when 'Ref' is resolved"),
-                    path=deque(["Fn::Sub"]),
-                    schema_path=deque([]),
+                    "{'Fn::GetAZs': ''} is not of type 'string'",
+                    path=deque(["Fn::Sub", 1, "foo"]),
+                    schema_path=deque(
+                        ["fn_items", "patternProperties", "[a-zA-Z0-9]+", "fn_getazs"]
+                    ),
                     validator="fn_sub",
                 ),
             ],
@@ -305,7 +322,14 @@ def test_validate(name, instance, schema, expected, rule, context, cfn):
         validators={
             "ref": Ref().ref,
             "fn_getatt": GetAtt().fn_getatt,
+            "fn_getazs": GetAz().fn_getazs,
         }
     )(context=context, cfn=cfn)
     errs = list(rule.fn_sub(validator, schema, instance, {}))
+
+    for err in errs:
+        print(err.message)
+        print(err.validator)
+        print(err.path)
+        print(err.schema_path)
     assert errs == expected, f"Test {name!r} got {errs!r}"