fix(security): replace unsafe expression evaluator with simpleeval

Python's built-in eval() with empty globals auto-injects __builtins__,
making __import__ accessible from user-controlled JSON config files.
Replace with simpleeval which only allows arithmetic, comparisons,
dict lookups, and whitelisted functions (max, min, round, abs, len, log).

Closes #1405

Signed-off-by: Yash Goel <yashhzd@gmail.com>
Signed-off-by: Yash Goel <yashhzd@users.noreply.github.com>

Author: yashhzd

ardupilot_methodic_configurator/backend_filesystem_configuration_steps.py

@@ -20,9 +20,11 @@
 # from logging import debug as logging_debug
 from jsonschema import validate as json_validate
 from jsonschema.exceptions import ValidationError
+from simpleeval import InvalidExpression
 
 from ardupilot_methodic_configurator import _
 from ardupilot_methodic_configurator.data_model_par_dict import Par, ParDict
+from ardupilot_methodic_configurator.safe_expression_evaluator import safe_evaluate
 
 
 class PhaseData(TypedDict, total=False):
@@ -193,7 +195,7 @@ def compute_parameters(  # pylint: disable=too-many-branches, too-many-arguments
                     continue
 
                 try:
-                    result = eval(str(parameter_info["New Value"]), {}, variables)  # noqa: S307 pylint: disable=eval-used
+                    result = safe_evaluate(str(parameter_info["New Value"]), variables)
                 except (ZeroDivisionError, ValueError):
                     # Handle math errors like:
                     # - ZeroDivisionError: division by zero or 0.0 raised to negative power
@@ -236,7 +238,7 @@ def compute_parameters(  # pylint: disable=too-many-branches, too-many-arguments
                     destination[filename] = ParDict()
                 change_reason = _(parameter_info["Change Reason"]) if parameter_info["Change Reason"] else ""
                 destination[filename][parameter] = Par(float(result), change_reason)
-            except (SyntaxError, NameError, KeyError, StopIteration) as _e:
+            except (SyntaxError, NameError, KeyError, StopIteration, InvalidExpression) as _e:
                 error_msg = _(
                     "In file '{self.configuration_steps_filename}': '{filename}' {parameter_type} "
                     "parameter '{parameter}' could not be computed: {_e}"

ardupilot_methodic_configurator/configuration_steps_ArduCopter.json

@@ -207,7 +207,7 @@
                 "ATC_RAT_RLL_FLTT": { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0))) / 2", "Change Reason": "INS_GYRO_FILTER / 2" },
                 "ATC_RAT_YAW_FLTT": { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0))) / 2", "Change Reason": "INS_GYRO_FILTER / 2" },
                 "INS_GYRO_FILTER":  { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0)))", "Change Reason": "Derived from vehicle component editor propeller size" },
-                "MOT_THST_EXPO":    { "New Value": "min(0.8, round(0.15686*__import__('math').log(vehicle_components['Propellers']['Specifications']['Diameter_inches'])+0.23693, 2))", "Change Reason": "Derived from vehicle component editor propeller size" }
+                "MOT_THST_EXPO":    { "New Value": "min(0.8, round(0.15686*log(vehicle_components['Propellers']['Specifications']['Diameter_inches'])+0.23693, 2))", "Change Reason": "Derived from vehicle component editor propeller size" }
             },
             "old_filenames": []
         },

ardupilot_methodic_configurator/configuration_steps_ArduPlane.json

@@ -206,7 +206,7 @@
                 "ATC_RAT_RLL_FLTT": { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0))) / 2", "Change Reason": "INS_GYRO_FILTER / 2" },
                 "ATC_RAT_YAW_FLTT": { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0))) / 2", "Change Reason": "INS_GYRO_FILTER / 2" },
                 "INS_GYRO_FILTER":  { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0)))", "Change Reason": "Derived from vehicle component editor propeller size" },
-                "MOT_THST_EXPO":    { "New Value": "min(0.8, round(0.15686*__import__('math').log(vehicle_components['Propellers']['Specifications']['Diameter_inches'])+0.23693, 2))", "Change Reason": "Derived from vehicle component editor propeller size" }
+                "MOT_THST_EXPO":    { "New Value": "min(0.8, round(0.15686*log(vehicle_components['Propellers']['Specifications']['Diameter_inches'])+0.23693, 2))", "Change Reason": "Derived from vehicle component editor propeller size" }
             },
             "old_filenames": []
         },

ardupilot_methodic_configurator/configuration_steps_Rover.json

@@ -206,7 +206,7 @@
                 "ATC_RAT_RLL_FLTT": { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0))) / 2", "Change Reason": "INS_GYRO_FILTER / 2" },
                 "ATC_RAT_YAW_FLTT": { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0))) / 2", "Change Reason": "INS_GYRO_FILTER / 2" },
                 "INS_GYRO_FILTER":  { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0)))", "Change Reason": "Derived from vehicle component editor propeller size" },
-                "MOT_THST_EXPO":    { "New Value": "min(0.8, round(0.15686*__import__('math').log(vehicle_components['Propellers']['Specifications']['Diameter_inches'])+0.23693, 2))", "Change Reason": "Derived from vehicle component editor propeller size" }
+                "MOT_THST_EXPO":    { "New Value": "min(0.8, round(0.15686*log(vehicle_components['Propellers']['Specifications']['Diameter_inches'])+0.23693, 2))", "Change Reason": "Derived from vehicle component editor propeller size" }
             },
             "old_filenames": []
         },

ardupilot_methodic_configurator/data_model_configuration_step.py

@@ -18,6 +18,7 @@
 from ardupilot_methodic_configurator.backend_filesystem import LocalFilesystem
 from ardupilot_methodic_configurator.data_model_ardupilot_parameter import ArduPilotParameter
 from ardupilot_methodic_configurator.data_model_par_dict import Par, ParDict
+from ardupilot_methodic_configurator.safe_expression_evaluator import safe_evaluate
 
 
 class ConfigurationStepProcessor:
@@ -299,7 +300,7 @@ def _calculate_connection_rename_operations(
         """
         if variables:
             # If variables provided, evaluate the new_connection_prefix
-            new_connection_prefix = eval(str(new_connection_prefix), {}, variables)  # noqa: S307 pylint: disable=eval-used
+            new_connection_prefix = safe_evaluate(str(new_connection_prefix), variables)
 
         # Generate the rename mapping
         renames = ConfigurationStepProcessor._generate_connection_renames(list(parameters.keys()), new_connection_prefix)

ardupilot_methodic_configurator/safe_expression_evaluator.py

@@ -0,0 +1,52 @@
+"""
+Safe expression evaluator for configuration step parameter processing.
+
+Replaces Python's built-in expression evaluation with simpleeval to prevent
+arbitrary code execution from user-controlled JSON configuration files.
+
+This file is part of ArduPilot Methodic Configurator. https://github.com/ArduPilot/MethodicConfigurator
+
+SPDX-FileCopyrightText: 2024-2026 Amilcar do Carmo Lucas <amilcar.lucas@iav.de>
+
+SPDX-License-Identifier: GPL-3.0-or-later
+"""
+
+from math import log
+from types import MappingProxyType
+
+from simpleeval import simple_eval
+
+SAFE_FUNCTIONS = MappingProxyType(
+    {
+        "max": max,
+        "min": min,
+        "round": round,
+        "abs": abs,
+        "len": len,
+        "log": log,
+    }
+)
+
+
+def safe_evaluate(expression: str, variables: dict) -> object:
+    """
+    Evaluate a parameter expression safely using simpleeval.
+
+    Only arithmetic, comparisons, ternary conditionals, dict lookups,
+    and whitelisted functions (max, min, round, abs, len, log) are allowed.
+    Any attempt to call __import__, access dunder attributes, or use
+    disallowed constructs will raise an exception.
+
+    Args:
+        expression: The expression string from a configuration step JSON file.
+        variables: A dictionary of variable names available during evaluation.
+
+    Returns:
+        The result of evaluating the expression.
+
+    Raises:
+        InvalidExpression: If the expression uses a disallowed feature,
+            calls an unknown function, or references an undefined variable.
+
+    """
+    return simple_eval(expression, names=variables, functions=SAFE_FUNCTIONS)

ardupilot_methodic_configurator/vehicle_templates/ArduPlane/normal_plane/configuration_steps_ArduPlane.json

@@ -199,7 +199,7 @@
                 "ATC_RAT_RLL_FLTT": { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0))) / 2", "Change Reason": "INS_GYRO_FILTER / 2" },
                 "ATC_RAT_YAW_FLTT": { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0))) / 2", "Change Reason": "INS_GYRO_FILTER / 2" },
                 "INS_GYRO_FILTER":  { "New Value": "max(20, (round(289.22*vehicle_components['Propellers']['Specifications']['Diameter_inches']**-0.838, 0)))", "Change Reason": "Derived from vehicle component editor propeller size" },
-                "MOT_THST_EXPO":    { "New Value": "min(0.8, round(0.15686*__import__('math').log(vehicle_components['Propellers']['Specifications']['Diameter_inches'])+0.23693, 2))", "Change Reason": "Derived from vehicle component editor propeller size" }
+                "MOT_THST_EXPO":    { "New Value": "min(0.8, round(0.15686*log(vehicle_components['Propellers']['Specifications']['Diameter_inches'])+0.23693, 2))", "Change Reason": "Derived from vehicle component editor propeller size" }
             },
             "old_filenames": []
         },

pyproject.toml

@@ -59,6 +59,7 @@ dependencies = [
     "pymavlink==2.4.49",
     "pyserial==3.5",
     "requests==2.32.5",
+    "simpleeval==1.0.7",
     "screeninfo==0.8.1",
 ]
 

tests/test_backend_filesystem_configuration_steps.py

@@ -463,5 +463,61 @@ def test_validate_parameters_empty_dict() -> None:
     assert True  # If we get here, no exception was raised
 
 
+def test_malicious_expression_import_blocked() -> None:
+    """Test that __import__ expressions are blocked by the safe evaluator."""
+    config_steps = ConfigurationSteps("vehicle_dir", "vehicle_type")
+    file_info = {
+        "forced_parameters": {
+            "PARAM1": {"New Value": "__import__('os').system('echo pwned')", "Change Reason": "Malicious"},
+        }
+    }
+    variables = {"doc_dict": {"PARAM1": {"values": {}}}}
+
+    result = config_steps.compute_parameters("test_file", file_info, "forced", variables)
+
+    assert result != ""
+    assert "PARAM1" in result
+    assert "test_file" not in config_steps.forced_parameters
+
+
+def test_malicious_expression_builtins_blocked() -> None:
+    """Test that accessing __builtins__ is blocked by the safe evaluator."""
+    config_steps = ConfigurationSteps("vehicle_dir", "vehicle_type")
+    file_info = {
+        "forced_parameters": {
+            "PARAM1": {"New Value": "__builtins__.__import__('os').system('id')", "Change Reason": "Malicious"},
+        }
+    }
+    variables = {"doc_dict": {"PARAM1": {"values": {}}}}
+
+    result = config_steps.compute_parameters("test_file", file_info, "forced", variables)
+
+    assert result != ""
+    assert "PARAM1" in result
+    assert "test_file" not in config_steps.forced_parameters
+
+
+def test_safe_log_function_in_expression() -> None:
+    """Test that log() works as a whitelisted safe function (replacing __import__('math').log())."""
+    config_steps = ConfigurationSteps("vehicle_dir", "vehicle_type")
+    file_info = {
+        "forced_parameters": {
+            "MOT_THST_EXPO": {
+                "New Value": "min(0.8, round(0.15686*log(vehicle_components['Propellers']['Specifications']['Diameter_inches'])+0.23693, 2))",
+                "Change Reason": "Derived from propeller size",
+            },
+        }
+    }
+    variables = {
+        "doc_dict": {"MOT_THST_EXPO": {"values": {}}},
+        "vehicle_components": {"Propellers": {"Specifications": {"Diameter_inches": 10}}},
+    }
+
+    result = config_steps.compute_parameters("test_file", file_info, "forced", variables)
+
+    assert result == ""
+    assert abs(config_steps.forced_parameters["test_file"]["MOT_THST_EXPO"].value - 0.6) < 0.01
+
+
 if __name__ == "__main__":
     unittest.main()