From a34a380c97dada5b908aba7431a31dc092c6ff15 Mon Sep 17 00:00:00 2001 From: Saar Ettinger Date: Fri, 17 Apr 2026 01:36:29 +0300 Subject: [PATCH] fix(general): Safe asteval rendering --- .../variable_rendering/safe_eval_functions.py | 7 ++++++- .../test_string_evaluation.py | 20 +++++++++++++++++-- 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/checkov/terraform/graph_builder/variable_rendering/safe_eval_functions.py b/checkov/terraform/graph_builder/variable_rendering/safe_eval_functions.py index db7c3ff4d9..0e55b7544e 100644 --- a/checkov/terraform/graph_builder/variable_rendering/safe_eval_functions.py +++ b/checkov/terraform/graph_builder/variable_rendering/safe_eval_functions.py @@ -355,13 +355,18 @@ def terraform_try(*args: Any) -> Any: SAFE_EVAL_DICT["timeadd"] = timeadd SAFE_EVAL_DICT["formatdate"] = formatdate +# Computed once at import time; passed to every Interpreter so asteval cannot +# overwrite TF builtins in the shared SAFE_EVAL_DICT via node_assign. +_SAFE_EVAL_READONLY: frozenset = frozenset(SAFE_EVAL_DICT) + def get_asteval() -> Interpreter: # asteval provides a safer environment for evaluating expressions by restricting the operations to a secure subset, significantly reducing the risk of executing malicious code. return Interpreter( symtable=SAFE_EVAL_DICT, use_numpy=False, - minimal=True + minimal=True, + readonly_symbols=_SAFE_EVAL_READONLY ) diff --git a/tests/terraform/graph/variable_rendering/test_string_evaluation.py b/tests/terraform/graph/variable_rendering/test_string_evaluation.py index 683162d508..9e419b06ae 100644 --- a/tests/terraform/graph/variable_rendering/test_string_evaluation.py +++ b/tests/terraform/graph/variable_rendering/test_string_evaluation.py @@ -7,7 +7,8 @@ from checkov.terraform.graph_builder.variable_rendering.evaluate_terraform import evaluate_terraform, \ replace_string_value, \ remove_interpolation, _find_new_value_for_interpolation -from checkov.terraform.graph_builder.variable_rendering.safe_eval_functions import evaluate, get_asteval +from checkov.terraform.graph_builder.variable_rendering.safe_eval_functions import evaluate, get_asteval, \ + SAFE_EVAL_DICT class TestTerraformEvaluation(TestCase): @@ -594,4 +595,19 @@ def test_evaluate_malicious_code(description: str, input_str: str)-> None: assert result == expected asteval = get_asteval() asteval(input_str) - assert asteval.error \ No newline at end of file + assert asteval.error + + +def test_asteval_symtable_is_isolated_per_call() -> None: + # Attacker-controlled assignments must not clobber TF builtins in SAFE_EVAL_DICT. + # Specifically: a quote-breakout that delivers "merge = 0" to asteval must not + # overwrite the merge() builtin for subsequent evaluations (F-005). + before_merge = SAFE_EVAL_DICT["merge"] + + asteval = get_asteval() + asteval("merge = 0") + + # The builtin must still be the original callable — not 0 + assert SAFE_EVAL_DICT["merge"] is before_merge + assert callable(SAFE_EVAL_DICT["merge"]) + assert evaluate('merge({"a": 1}, {"b": 2})') == {"a": 1, "b": 2}