Python: Replace eval with safer alternatives

gui/wxpython/core/menutree.py

@@ -114,7 +114,7 @@ def _createItem(self, item, node):
                 "" if keywords is None or keywords.text is None else keywords.text
             )
             shortcut = shortcut.text if shortcut is not None else ""
-            wxId = eval("wx." + wxId.text) if wxId is not None else wx.ID_ANY
+            wxId = getattr(wx, wxId.text, wx.ID_ANY) if wxId is not None else wx.ID_ANY
             icon = icon.text if icon is not None else ""
             label = origLabel
             if gcmd:

gui/wxpython/gmodeler/model.py

@@ -734,7 +734,10 @@ def Run(self, log, onDone, parent=None):
                     if ret:
                         vlist = ret.splitlines()
                 else:
-                    vlist = eval(condText)
+                    # The loop condition is part of the model authored by the
+                    # user, so evaluating it is equivalent to running the
+                    # model itself.
+                    vlist = eval(condText)  # nosec B307
 
                 if "variables" not in params:
                     params["variables"] = {"params": []}

gui/wxpython/gui_core/menu.py

@@ -118,7 +118,7 @@ def _createMenuItem(
             ):
                 menuItem.Enable(False)
 
-        rhandler = eval("self.class_handler." + handler)  # nosec B307
+        rhandler = getattr(self.class_handler, handler)
         self.parent.Bind(wx.EVT_MENU, rhandler, menuItem)
 
     def GetData(self):
@@ -279,12 +279,12 @@ def Run(self, node=None):
             return
 
         # extract name of the handler and create a new call
-        handler = "self._handlerObj." + data["handler"].lstrip("self.")
+        handler = getattr(self._handlerObj, data["handler"].removeprefix("self."))
 
         if data["command"]:
-            eval(handler)(event=None, cmd=data["command"].split())
+            handler(event=None, cmd=data["command"].split())
         else:
-            eval(handler)(event=None)
+            handler(event=None)
 
     def Help(self, node=None):
         """Show documentation for a module"""

python/grass/temporal/temporal_algebra.py

@@ -1690,6 +1690,12 @@ def compare_bool_value(
         # Build conditional list with elements from related maps and given relation
         # operator.
         leftbool = map_i.condition_value[0]
+        # The seed value and every value appended in the loop below must be a
+        # bool, so the joined string contains only bool literals and grammar
+        # tokens (see the eval justification below).
+        if not isinstance(leftbool, bool):
+            msg = "Condition value must be a bool, got {}".format(type(leftbool))
+            raise TypeError(msg)
         condition_value_list = [leftbool]
         count = 0
         for topo in temporal_topo_list:
@@ -1723,7 +1729,10 @@ def compare_bool_value(
         condition_value_str = "".join(map(str, condition_value_list))
         if self.debug:
             print(condition_value_str)
-        resultbool = eval(condition_value_str)
+        # condition_value_str is built above solely from bool values (the seed
+        # and loop values are all checked to be bool) joined with grammar
+        # tokens, so evaluating it does not run arbitrary input.
+        resultbool = eval(condition_value_str)  # nosec B307
         if self.debug:
             print(resultbool)
         # Add boolean value to result list.
@@ -2094,22 +2103,30 @@ def get_temporal_func_dict(self, map):
             #              "supported for absolute time." % (str(map.get_id()))))
         return tvardict
 
-    def eval_datetime_str(self, tfuncval, comp, value):
-        # Evaluate date object comparison expression.
+    def compare_values(self, left, comp, right):
+        """Compare two values with the given comparison operator."""
         if comp == "<":
-            boolname = eval(str(tfuncval < value))
-        elif comp == ">":
-            boolname = eval(str(tfuncval > value))
-        elif comp == "==":
-            boolname = eval(str(tfuncval == value))
-        elif comp == "<=":
-            boolname = eval(str(tfuncval <= value))
-        elif comp == ">=":
-            boolname = eval(str(tfuncval >= value))
-        elif comp == "!=":
-            boolname = eval(str(tfuncval != value))
-
-        return boolname
+            return left < right
+        if comp == ">":
+            return left > right
+        if comp == "==":
+            return left == right
+        if comp == "<=":
+            return left <= right
+        if comp == ">=":
+            return left >= right
+        if comp == "!=":
+            return left != right
+        msg = "Unknown comparison operator: {}".format(comp)
+        raise ValueError(msg)
+
+    def eval_datetime_str(self, tfuncval, comp, value):
+        """Compare two values with the given comparison operator.
+
+        .. deprecated:: 8.6.0
+            Use :func:`compare_values` instead.
+        """
+        return self.compare_values(tfuncval, comp, value)
 
     def eval_global_var(self, gvar, maplist):
         """This function evaluates a global variable expression for a map list.
@@ -2143,9 +2160,9 @@ def eval_global_var(self, gvar, maplist):
             }:
                 timeobj = string_to_datetime(value.replace('"', ""))
                 value = timeobj.date()
-                boolname = self.eval_datetime_str(tfuncval, comp_op, value)
-            else:
-                boolname = eval(str(tfuncval) + comp_op + str(value))
+            # value is already an int/float here (from t_INT/t_FLOAT) or a date
+            # from the branch above, so it is compared without coercion.
+            boolname = self.compare_values(tfuncval, comp_op, value)
             # Add conditional boolean value to the map.
             if "condition_value" in dir(map_i):
                 map_i.condition_value.append(boolname)
@@ -2824,18 +2841,19 @@ def p_t_var_expr_td_hash(self, t) -> None:
         if self.run:
             maplist = self.check_stds(t[1])
             comp_op = t[2]
-            value = str(t[3])
+            # t[3] is already an int/float from the lexer (t_INT/t_FLOAT).
+            value = t[3]
             for map_i in maplist:
                 # Evaluate time differences and hash operator statements for each map.
                 try:
                     td = map_i.map_value[0].td
-                    boolname = eval(str(td) + comp_op + value)
+                    boolname = self.compare_values(td, comp_op, value)
                     # Add conditional boolean value to the map.
                     if "condition_value" in dir(map_i):
                         map_i.condition_value.append(boolname)
                     else:
                         map_i.condition_value = boolname
-                except (IndexError, AttributeError, SyntaxError):
+                except (IndexError, AttributeError, TypeError):
                     self.msgr.fatal(
                         "Error: the given expression does not contain a correct time "
                         "difference object."

python/grass/temporal/tests/temporal_algebra_compare_values_test.py

@@ -0,0 +1,55 @@
+"""Tests of TemporalAlgebraParser.compare_values.
+
+These make the comparison behavior of the temporal algebra observable, in
+particular the ``!=`` operator, which no expression-level test exercises.
+"""
+
+import datetime
+from types import SimpleNamespace
+
+import pytest
+
+from grass.temporal.temporal_algebra import TemporalAlgebraParser
+
+# compare_values does not use instance state, so build an instance without
+# running __init__ (which would open a temporal database connection). Provide a
+# stub dbif so __del__ (which checks dbif.connected) stays a no-op.
+parser = TemporalAlgebraParser.__new__(TemporalAlgebraParser)
+parser.dbif = SimpleNamespace(connected=False)
+
+
+@pytest.mark.parametrize(
+    ("left", "right"),
+    [
+        (5, 2),  # int
+        (2, 2),  # equal ints
+        (5.0, 2.5),  # float
+        (datetime.date(2001, 1, 1), datetime.date(2000, 1, 1)),  # date
+        (datetime.date(2001, 1, 1), datetime.date(2001, 1, 1)),  # equal dates
+    ],
+)
+def test_compare_values_matches_python(left, right):
+    """Each operator returns the plain Python comparison result."""
+    assert parser.compare_values(left, "<", right) == (left < right)
+    assert parser.compare_values(left, ">", right) == (left > right)
+    assert parser.compare_values(left, "==", right) == (left == right)
+    assert parser.compare_values(left, "!=", right) == (left != right)
+    assert parser.compare_values(left, "<=", right) == (left <= right)
+    assert parser.compare_values(left, ">=", right) == (left >= right)
+
+
+def test_compare_values_not_equal():
+    """The != branch, otherwise unexercised, behaves as expected."""
+    assert parser.compare_values(1, "!=", 2) is True
+    assert parser.compare_values(2, "!=", 2) is False
+
+
+def test_compare_values_keeps_int_type():
+    """No float coercion: integer comparisons stay exact."""
+    assert parser.compare_values(2001, "==", 2001) is True
+    assert parser.compare_values(3, "<", 10) is True
+
+
+def test_compare_values_unknown_operator():
+    with pytest.raises(ValueError, match="Unknown comparison operator"):
+        parser.compare_values(1, "=<", 2)

scripts/i.tasscap/i.tasscap.py

@@ -326,7 +326,7 @@ def calcN(outpre, bands, satel):
         bands_num = used_bands[i]
 
         # use combination function suitable for used number of bands
-        eval("calc1bands%d(out, bands, *p)" % bands_num)
+        globals()["calc1bands%d" % bands_num](out, bands, *p)
         gs.run_command("r.colors", map=out, color="grey", quiet=True)