Skip to content

Commit 4a06a85

Browse files
committed
Revert "fix(general): using asteval instead of using eval (#7116)"
This reverts commit 5ddad08.
1 parent e508931 commit 4a06a85

File tree

8 files changed

+35
-108
lines changed

8 files changed

+35
-108
lines changed

Pipfile

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,6 @@ spdx-tools = ">=0.8.0,<0.9.0"
8484
license-expression = ">=30.1.0,<31.0.0"
8585
rustworkx = ">=0.13.0,<1.0.0"
8686
pydantic = ">=2.0.0,<3.0.0"
87-
asteval = "==1.0.5"
8887

8988
[requires]
9089
python_version = "3.8"

Pipfile.lock

Lines changed: 20 additions & 21 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

checkov/terraform/graph_builder/variable_rendering/evaluate_terraform.py

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
# exclude "']" one the right side of the compare via (?!']), this can happen with a base64 encoded string
2121
COMPARE_REGEX = re.compile(r"^(?P<a>.+?)\s*(?P<operator>==|!=|>=|>|<=|<|&&|\|\|)\s*(?P<b>(?!']).+)$")
2222
COMPARE_OPERATORS = (" == ", " != ", " < ", " <= ", " > ", " >= ", " && ", " || ")
23-
REMOVE_TRAILING_COMMAS = re.compile(r',(\s*[}\]])')
2423

2524
CHECKOV_RENDER_MAX_LEN = force_int(os.getenv("CHECKOV_RENDER_MAX_LEN", "10000"))
2625

@@ -85,10 +84,7 @@ def _eval_merge_as_list(eval_value: Any) -> Any:
8584

8685
def _try_evaluate(input_str: Union[str, bool]) -> Any:
8786
try:
88-
result = evaluate(input_str) # type:ignore[arg-type]
89-
if result is None:
90-
raise Exception(f"Can't evaluate {input_str}")
91-
return result
87+
return evaluate(input_str) # type:ignore[arg-type]
9288
except Exception:
9389
try:
9490
return evaluate(f'"{input_str}"')
@@ -101,12 +97,7 @@ def _try_evaluate(input_str: Union[str, bool]) -> Any:
10197
return json.loads(input_str)
10298
return input_str
10399
except Exception:
104-
try:
105-
# Remove trailing commas before } or ]
106-
input_str_no_trailing = REMOVE_TRAILING_COMMAS.sub(r'\1', input_str) # type:ignore[arg-type]
107-
return json.loads(input_str_no_trailing)
108-
except Exception:
109-
return input_str
100+
return input_str
110101

111102

112103
def replace_string_value(original_str: Any, str_to_replace: str, replaced_value: str, keep_origin: bool = True) -> Any:

checkov/terraform/graph_builder/variable_rendering/safe_eval_functions.py

Lines changed: 8 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,6 @@
88
from functools import reduce
99
from math import ceil, floor, log
1010
from typing import Union, Any, Dict, Callable, List, Optional
11-
from asteval import Interpreter
1211

1312
from checkov.terraform.parser_functions import tonumber, FUNCTION_FAILED, create_map, tobool, tostring
1413

@@ -357,21 +356,13 @@ def terraform_try(*args: Any) -> Any:
357356
SAFE_EVAL_DICT["formatdate"] = formatdate
358357

359358

360-
def get_asteval() -> Interpreter:
361-
# asteval provides a safer environment for evaluating expressions by restricting the operations to a secure subset, significantly reducing the risk of executing malicious code.
362-
return Interpreter(
363-
symtable=SAFE_EVAL_DICT,
364-
use_numpy=False,
365-
minimal=True
366-
)
367-
368-
369359
def evaluate(input_str: str) -> Any:
370-
"""
371-
Safely evaluate a Terraform-like function expression using a predefined function map.
372-
Falls back gracefully if evaluation fails.
373-
"""
374-
if not input_str or input_str == "...":
360+
count_underscores = input_str.count("__")
361+
# We are operating under the assumption that the function name will start and end with "__", ensuring that we have at least two of them
362+
if count_underscores >= 2:
363+
logging.debug(f"got a substring with double underscore, which is not allowed. origin string: {input_str}")
364+
return input_str
365+
if input_str == "...":
375366
# don't create an Ellipsis object
376367
return input_str
377368
if input_str.startswith("try"):
@@ -380,19 +371,11 @@ def evaluate(input_str: str) -> Any:
380371

381372
# Don't use str.replace to make sure we replace just the first occurrence
382373
input_str = f"{TRY_STR_REPLACEMENT}{input_str[3:]}"
383-
asteval = get_asteval()
384-
log_level = os.getenv("LOG_LEVEL")
385-
should_log_asteval_errors = log_level == "DEBUG"
386374
if RANGE_PATTERN.match(input_str):
387-
temp_eval = asteval(input_str, show_errors=should_log_asteval_errors)
375+
temp_eval = eval(input_str, {"__builtins__": None}, SAFE_EVAL_DICT) # nosec
388376
evaluated = input_str if temp_eval < 0 else temp_eval
389377
else:
390-
evaluated = asteval(input_str, show_errors=should_log_asteval_errors)
391-
392-
if asteval.error:
393-
error_messages = [err.get_error() for err in asteval.error]
394-
raise ValueError(f"Safe evaluation error: {error_messages}")
395-
378+
evaluated = eval(input_str, {"__builtins__": None}, SAFE_EVAL_DICT) # nosec
396379
return evaluated if not isinstance(evaluated, str) else remove_unicode_null(evaluated)
397380

398381

mypy.ini

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,3 @@ ignore_missing_imports = True
1919

2020
[mypy-checkov.*]
2121
follow_imports = skip
22-
23-
[mypy-asteval.*]
24-
ignore_missing_imports = True

performance_tests/test_checkov_performance.py

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
'repo_name': 'terraform-aws-components',
1919
'threshold': {
2020
"Darwin": 19.0,
21-
"Linux": 13.0,
21+
"Linux": 11.0,
2222
"Windows": 15.0,
2323
}
2424
},
@@ -114,5 +114,6 @@ def run_kubernetes_scan():
114114
runner_registry = RunnerRegistry(banner, runner_filter, k8_runner())
115115
reports = runner_registry.run(root_folder=test_files_dir)
116116
assert len(reports) > 0
117+
117118
benchmark(run_kubernetes_scan)
118119
assert benchmark.stats.stats.mean <= repo_threshold + (DEVIATION_PERCENT / 100) * repo_threshold

setup.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,6 @@ def run(self) -> None:
107107
"license-expression<31.0.0,>=30.1.0",
108108
"rustworkx>=0.13.0,<1.0.0",
109109
"pydantic<3.0.0,>=2.0.0",
110-
"asteval==1.0.5"
111110
],
112111
dependency_links=[], # keep it empty, needed for pipenv-setup
113112
license="Apache License 2.0",

tests/terraform/graph/variable_rendering/test_string_evaluation.py

Lines changed: 3 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
from checkov.terraform.graph_builder.variable_rendering.evaluate_terraform import evaluate_terraform, \
88
replace_string_value, \
99
remove_interpolation, _find_new_value_for_interpolation
10-
from checkov.terraform.graph_builder.variable_rendering.safe_eval_functions import evaluate, get_asteval
10+
from checkov.terraform.graph_builder.variable_rendering.safe_eval_functions import evaluate
1111

1212

1313
class TestTerraformEvaluation(TestCase):
@@ -518,29 +518,11 @@ def test_try_then_merge_block(self):
518518
result = evaluate_terraform(input_str)
519519
self.assertEqual(expected, result)
520520

521-
def test_empty_string(self):
522-
input_str = " "
523-
expected = input_str
524-
result = evaluate_terraform(input_str)
525-
self.assertEqual(expected, result)
526-
527-
input_str = ""
528-
expected = input_str
529-
result = evaluate_terraform(input_str)
530-
self.assertEqual(expected, result)
531-
532-
def test_dict_as_string(self):
533-
expected = {'Statement': [
534-
{'Action': ['lambda:CreateFunction', 'lambda:CreateEventSourceMapping', 'dynamodb:CreateTable'],
535-
'Effect': 'Allow', 'Resource': '*'}], 'Version': '2012-10-17'}
536-
input_str = ' { "Version": "2012-10-17", "Statement": [ { "Effect": "Allow", "Action": [ "lambda:CreateFunction", "lambda:CreateEventSourceMapping", "dynamodb:CreateTable", ], "Resource": "*" } ] }'
537-
result = evaluate_terraform(input_str)
538-
assert result == expected
539-
540521

541522
@pytest.mark.parametrize(
542523
"origin_str,str_to_replace,new_value,expected",
543524
[
525+
544526
("${lookup({'a': ${local.protocol1}},\"a\",\"https\")}", '${local.protocol1}', 'local.protocol1', "'local.protocol1'"),
545527
('${length(keys(var.identity)) > 0 ? [${var.identity}] : []}', '${var.identity}', 'var.identity', 'var.identity'),
546528
],
@@ -555,32 +537,8 @@ def test_evaluate_range_pattern() -> None:
555537

556538
# Test range pattern
557539
assert evaluate("1-10") == "1-10"
558-
assert evaluate("5-25")== "5-25"
540+
assert evaluate("5-25") == "5-25"
559541
assert evaluate("10-5") == 5
560542

561543
# Test non-range pattern for comparison
562544
assert evaluate("1+1") == 2
563-
564-
565-
EVAL_DANGEROUS_INPUTS = [
566-
("unicode_characters", "[{}._﹎class_﹎._﹎bases_﹎[0]._﹎subclasses_﹎()]"),
567-
("generators_to_bypass_protections_code", "((gen:=(gen_list[0].gi_frame.f_back.f_back.f_globals for _ in [1,]),v:=gen_list.append(gen), gen_list[0])[2] for gen_list in [[]]).send(None).send(None).get('_'+'_builtins_'+'_')['_'+'_import_'+'_']('os').system('date >> /tmp/gen-example')"),
568-
("system command", "__import__('os').system('rm -rf /')"),
569-
("read passwd", "open('/etc/passwd').read()"),
570-
("access builtins", "__builtins__.__import__('os').listdir('.')"),
571-
("exec call", "exec('print(123)')"),
572-
("create file", "__import__('os').popen('touch /tmp/hacked').read()"),
573-
("subclasses", "().__class__.__bases__[0].__subclasses__()"),
574-
("socket connection", "__import__('socket').socket().connect(('example.com', 80))"),
575-
("lambda trick", "(lambda f: f('ls'))(__import__('os').system)")
576-
]
577-
578-
579-
@pytest.mark.parametrize("description, input_str", EVAL_DANGEROUS_INPUTS)
580-
def test_evaluate_malicious_code(description: str, input_str: str)-> None:
581-
expected = input_str
582-
result = evaluate_terraform(input_str)
583-
assert result == expected
584-
asteval = get_asteval()
585-
asteval(input_str)
586-
assert asteval.error

0 commit comments

Comments
 (0)