From c8adc32de0ac5baf5e5b71f62dd81b4ed934d91b Mon Sep 17 00:00:00 2001 From: cwhite911 Date: Tue, 14 Jul 2026 10:56:16 -0400 Subject: [PATCH 1/3] Python: Replace eval with safer alternatives In temporal algebra, compare values directly instead of eval of stringified comparisons, and route the td and global-variable comparisons through the same helper; the remaining eval of the internally built boolean string is documented and marked nosec. In the wxGUI, resolve wx ids and menu handlers with getattr instead of eval, and document the gmodeler loop-condition eval, which runs the model author's own expression. In i.tasscap, dispatch to the band combination function via globals() lookup. Fixes 14 Bandit B307 (eval) alerts. --- gui/wxpython/core/menutree.py | 2 +- gui/wxpython/gmodeler/model.py | 5 ++++- gui/wxpython/gui_core/menu.py | 8 ++++---- python/grass/temporal/temporal_algebra.py | 24 ++++++++++++----------- scripts/i.tasscap/i.tasscap.py | 2 +- 5 files changed, 23 insertions(+), 18 deletions(-) diff --git a/gui/wxpython/core/menutree.py b/gui/wxpython/core/menutree.py index a418ffcd98a..9e24c44adbe 100644 --- a/gui/wxpython/core/menutree.py +++ b/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) if wxId is not None else wx.ID_ANY icon = icon.text if icon is not None else "" label = origLabel if gcmd: diff --git a/gui/wxpython/gmodeler/model.py b/gui/wxpython/gmodeler/model.py index f8e3525cac6..8430835ec56 100644 --- a/gui/wxpython/gmodeler/model.py +++ b/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": []} diff --git a/gui/wxpython/gui_core/menu.py b/gui/wxpython/gui_core/menu.py index 876b56a4ec8..631c9b9e1fc 100644 --- a/gui/wxpython/gui_core/menu.py +++ b/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"].lstrip("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""" diff --git a/python/grass/temporal/temporal_algebra.py b/python/grass/temporal/temporal_algebra.py index 37da1099bc9..8cc838c1b0e 100644 --- a/python/grass/temporal/temporal_algebra.py +++ b/python/grass/temporal/temporal_algebra.py @@ -1723,7 +1723,9 @@ 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) + # The string is built above solely from checked bool values and + # 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. @@ -2095,19 +2097,19 @@ def get_temporal_func_dict(self, map): return tvardict def eval_datetime_str(self, tfuncval, comp, value): - # Evaluate date object comparison expression. + # Evaluate a comparison of two values with the given operator. if comp == "<": - boolname = eval(str(tfuncval < value)) + boolname = tfuncval < value elif comp == ">": - boolname = eval(str(tfuncval > value)) + boolname = tfuncval > value elif comp == "==": - boolname = eval(str(tfuncval == value)) + boolname = tfuncval == value elif comp == "<=": - boolname = eval(str(tfuncval <= value)) + boolname = tfuncval <= value elif comp == ">=": - boolname = eval(str(tfuncval >= value)) + boolname = tfuncval >= value elif comp == "!=": - boolname = eval(str(tfuncval != value)) + boolname = tfuncval != value return boolname @@ -2145,7 +2147,7 @@ def eval_global_var(self, gvar, maplist): value = timeobj.date() boolname = self.eval_datetime_str(tfuncval, comp_op, value) else: - boolname = eval(str(tfuncval) + comp_op + str(value)) + boolname = self.eval_datetime_str(tfuncval, comp_op, float(value)) # Add conditional boolean value to the map. if "condition_value" in dir(map_i): map_i.condition_value.append(boolname) @@ -2829,13 +2831,13 @@ def p_t_var_expr_td_hash(self, t) -> None: # 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.eval_datetime_str(td, comp_op, float(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, ValueError): self.msgr.fatal( "Error: the given expression does not contain a correct time " "difference object." diff --git a/scripts/i.tasscap/i.tasscap.py b/scripts/i.tasscap/i.tasscap.py index fb22c6048ed..0c2f105db49 100755 --- a/scripts/i.tasscap/i.tasscap.py +++ b/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) From 5a0d9d625b2fdb0a0cf270715275901444fc8399 Mon Sep 17 00:00:00 2001 From: Corey White Date: Fri, 17 Jul 2026 08:38:11 -0400 Subject: [PATCH 2/3] Update gui/wxpython/core/menutree.py --- gui/wxpython/core/menutree.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gui/wxpython/core/menutree.py b/gui/wxpython/core/menutree.py index 9e24c44adbe..57671bd94ab 100644 --- a/gui/wxpython/core/menutree.py +++ b/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 = getattr(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: From 48026e5e8a108bc3e6c4d6d67a096ec57131b89f Mon Sep 17 00:00:00 2001 From: cwhite911 Date: Fri, 17 Jul 2026 15:24:10 -0400 Subject: [PATCH 3/3] grass.temporal: Address review of eval replacements - Rename eval_datetime_str to compare_values (it no longer evals or takes strings) and keep a deprecated alias; add an explicit raise for an unknown operator instead of falling through to UnboundLocalError. - Drop the unnecessary float()/str() coercions at both call sites so integer comparisons stay int-exact, matching the old eval semantics and the documented direction of t_var comp_op t_var. - Add a unit test exercising all six operators over int/float/date, including != which no expression-level test reaches. - gui menu: use str.removeprefix('self.') instead of lstrip, which strips a character set rather than a prefix (latent bug). - Tighten the B307 nosec justification by guarding the seed condition value as a bool, so the claim that only booleans are evaluated holds. --- gui/wxpython/gui_core/menu.py | 2 +- python/grass/temporal/temporal_algebra.py | 62 ++++++++++++------- .../temporal_algebra_compare_values_test.py | 55 ++++++++++++++++ 3 files changed, 95 insertions(+), 24 deletions(-) create mode 100644 python/grass/temporal/tests/temporal_algebra_compare_values_test.py diff --git a/gui/wxpython/gui_core/menu.py b/gui/wxpython/gui_core/menu.py index 631c9b9e1fc..ef0ee343d43 100644 --- a/gui/wxpython/gui_core/menu.py +++ b/gui/wxpython/gui_core/menu.py @@ -279,7 +279,7 @@ def Run(self, node=None): return # extract name of the handler and create a new call - handler = getattr(self._handlerObj, data["handler"].lstrip("self.")) + handler = getattr(self._handlerObj, data["handler"].removeprefix("self.")) if data["command"]: handler(event=None, cmd=data["command"].split()) diff --git a/python/grass/temporal/temporal_algebra.py b/python/grass/temporal/temporal_algebra.py index 8cc838c1b0e..fa2cd975ca2 100644 --- a/python/grass/temporal/temporal_algebra.py +++ b/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,8 +1729,9 @@ def compare_bool_value( condition_value_str = "".join(map(str, condition_value_list)) if self.debug: print(condition_value_str) - # The string is built above solely from checked bool values and - # grammar tokens, so evaluating it does not run arbitrary input. + # 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) @@ -2096,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 a comparison of two values with the given operator. + def compare_values(self, left, comp, right): + """Compare two values with the given comparison operator.""" if comp == "<": - boolname = tfuncval < value - elif comp == ">": - boolname = tfuncval > value - elif comp == "==": - boolname = tfuncval == value - elif comp == "<=": - boolname = tfuncval <= value - elif comp == ">=": - boolname = tfuncval >= value - elif comp == "!=": - boolname = 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. @@ -2145,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 = self.eval_datetime_str(tfuncval, comp_op, float(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) @@ -2826,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 = self.eval_datetime_str(td, comp_op, float(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, TypeError, ValueError): + except (IndexError, AttributeError, TypeError): self.msgr.fatal( "Error: the given expression does not contain a correct time " "difference object." diff --git a/python/grass/temporal/tests/temporal_algebra_compare_values_test.py b/python/grass/temporal/tests/temporal_algebra_compare_values_test.py new file mode 100644 index 00000000000..beccdebf46f --- /dev/null +++ b/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)