security: replace unrestricted setattr with allowlist in Python backend

.agents/skills/python-kwargs-setattr-security/SKILL.md

@@ -0,0 +1,59 @@
+---
+name: python-kwargs-setattr-security
+description: When reviewing or fixing Python code that uses setattr() with user-controlled kwargs to configure C++ extension objects (SessionOptions, RunOptions, etc.) in ONNX Runtime. Use this to apply the allowlist pattern that prevents arbitrary file writes and other attacks via reflected property access.
+---
+
+## Problem Pattern
+
+Using `hasattr(obj, k) / setattr(obj, k, v)` with user-controlled kwargs is insecure. The `hasattr` check is NOT a security guard — it returns True for ALL exposed properties including dangerous ones.
+
+```python
+# INSECURE — do not use
+for k, v in kwargs.items():
+    if hasattr(options, k):
+        setattr(options, k, v)
+```
+
+## Fix: Explicit Allowlist
+
+Define a module-level frozenset of safe attribute names. Raise RuntimeError for known-but-blocked attrs; silently ignore unknown keys.
+
+```python
+# Define at module level, before the class
+_ALLOWED_SESSION_OPTIONS = frozenset({
+    "enable_cpu_mem_arena",
+    "enable_mem_pattern",
+    # ... only explicitly reviewed safe attrs
+})
+
+# In the method
+for k, v in kwargs.items():
+    if k in _ALLOWED_SESSION_OPTIONS:
+        setattr(options, k, v)
+    elif hasattr(options, k):  # reuse the existing instance, don't create new
+        raise RuntimeError(
+            f"SessionOptions attribute '{k}' is not permitted via the backend API. "
+            f"Allowed attributes: {sorted(_ALLOWED_SESSION_OPTIONS)}"
+        )
+    # else: silently ignore (may be kwargs for a different config object)
+```
+
+## Key Rules
+
+1. **Use the existing object** in `hasattr(options, k)` — never `hasattr(ClassName(), k)` (creates throwaway C++ objects per iteration)
+2. **RuntimeError** is the ORT convention for API misuse errors (not ValueError)
+3. **Silent ignore for RunOptions** when the same kwargs are forwarded from a SessionOptions path (e.g. `run_model()` sends kwargs to both `prepare` and `rep.run`)
+4. **Frozenset constant naming**: `_ALLOWED_<CLASSNAME>` — ALL_CAPS, Google Style
+5. **No type annotations** on module-level constants (ORT Python convention)
+
+## Dangerous SessionOptions Properties (never allowlist)
+
+- `optimized_model_filepath` — triggers Model::Save(), overwrites arbitrary files
+- `profile_file_prefix` + `enable_profiling` — writes profiling JSON to arbitrary path
+- `register_custom_ops_library` — loads arbitrary shared libraries (method, not property)
+
+## Files in ONNX Runtime
+
+- `onnxruntime/python/backend/backend.py` — `_ALLOWED_SESSION_OPTIONS`
+- `onnxruntime/python/backend/backend_rep.py` — `_ALLOWED_RUN_OPTIONS`
+- Tests: `onnxruntime/test/python/onnxruntime_test_python_backend.py` — `TestBackendKwargsAllowlist`

onnxruntime/python/backend/backend.py

@@ -15,7 +15,28 @@
 from onnx.checker import check_model
 
 from onnxruntime import InferenceSession, SessionOptions, get_available_providers, get_device
-from onnxruntime.backend.backend_rep import OnnxRuntimeBackendRep
+from onnxruntime.backend.backend_rep import OnnxRuntimeBackendRep, _ALLOWED_RUN_OPTIONS
+
+# Allowlist of SessionOptions attributes that are safe to set via the backend API.
+# Dangerous attributes (e.g. optimized_model_filepath, profile_file_prefix, enable_profiling)
+# are intentionally excluded to prevent arbitrary file writes through user-controlled kwargs.
+_ALLOWED_SESSION_OPTIONS = frozenset(
+    {
+        "enable_cpu_mem_arena",
+        "enable_mem_pattern",
+        "enable_mem_reuse",
+        "execution_mode",
+        "execution_order",
+        "graph_optimization_level",
+        "inter_op_num_threads",
+        "intra_op_num_threads",
+        "log_severity_level",
+        "log_verbosity_level",
+        "logid",
+        "use_deterministic_compute",
+        "use_per_session_threads",
+    }
+)
 
 
 class OnnxRuntimeBackend(Backend):
@@ -101,7 +122,8 @@ def prepare(cls, model, device=None, **kwargs):
         :param device: requested device for the computation,
             None means the default one which depends on
             the compilation settings
-        :param kwargs: see :class:`onnxruntime.SessionOptions`
+        :param kwargs: only a safe subset of :class:`onnxruntime.SessionOptions` attributes are
+            accepted; see ``_ALLOWED_SESSION_OPTIONS`` for the list
         :return: :class:`onnxruntime.InferenceSession`
         """
         if isinstance(model, OnnxRuntimeBackendRep):
@@ -111,8 +133,13 @@ def prepare(cls, model, device=None, **kwargs):
         elif isinstance(model, (str, bytes)):
             options = SessionOptions()
             for k, v in kwargs.items():
-                if hasattr(options, k):
+                if k in _ALLOWED_SESSION_OPTIONS:
                     setattr(options, k, v)
+                elif hasattr(options, k):
+                    raise RuntimeError(
+                        f"SessionOptions attribute '{k}' is not permitted via the backend API. "
+                        f"Allowed attributes: {', '.join(sorted(_ALLOWED_SESSION_OPTIONS))}"
+                    )
 
             excluded_providers = os.getenv("ORT_ONNX_BACKEND_EXCLUDE_PROVIDERS", default="").split(",")
             providers = [x for x in get_available_providers() if (x not in excluded_providers)]
@@ -154,11 +181,14 @@ def run_model(cls, model, inputs, device=None, **kwargs):
         :param device: requested device for the computation,
             None means the default one which depends on
             the compilation settings
-        :param kwargs: see :class:`onnxruntime.RunOptions`
+        :param kwargs: accepted keys are the union of ``_ALLOWED_SESSION_OPTIONS``
+            (see ``backend.py``) and ``_ALLOWED_RUN_OPTIONS`` (see ``backend_rep.py``)
         :return: predictions
         """
-        rep = cls.prepare(model, device, **kwargs)
-        return rep.run(inputs, **kwargs)
+        session_kwargs = {k: v for k, v in kwargs.items() if k in _ALLOWED_SESSION_OPTIONS}
+        run_kwargs = {k: v for k, v in kwargs.items() if k in _ALLOWED_RUN_OPTIONS}
+        rep = cls.prepare(model, device, **session_kwargs)
+        return rep.run(inputs, **run_kwargs)
 
     @classmethod
     def run_node(cls, node, inputs, device=None, outputs_info=None, **kwargs):

onnxruntime/python/backend/backend_rep.py

@@ -10,6 +10,18 @@
 
 from onnxruntime import RunOptions
 
+# Allowlist of RunOptions attributes that are safe to set via the backend API.
+# 'terminate' excluded: setting it True would deny the current inference call.
+# 'training_mode' excluded: silently switches inference behavior in training builds.
+_ALLOWED_RUN_OPTIONS = frozenset(
+    {
+        "log_severity_level",
+        "log_verbosity_level",
+        "logid",
+        "only_execute_path_to_fetches",
+    }
+)
+
 
 class OnnxRuntimeBackendRep(BackendRep):
     """
@@ -31,8 +43,14 @@ def run(self, inputs, **kwargs):  # type: (Any, **Any) -> Tuple[Any, ...]
 
         options = RunOptions()
         for k, v in kwargs.items():
-            if hasattr(options, k):
+            if k in _ALLOWED_RUN_OPTIONS:
                 setattr(options, k, v)
+            elif hasattr(options, k):
+                raise RuntimeError(
+                    f"RunOptions attribute '{k}' is not permitted via the backend API. "
+                    f"Allowed attributes: {', '.join(sorted(_ALLOWED_RUN_OPTIONS))}"
+                )
+            # else: silently ignore unknown keys
 
         if isinstance(inputs, list):
             inps = {}

onnxruntime/test/python/onnxruntime_test_python_backend.py

@@ -2,6 +2,8 @@
 # Licensed under the MIT License.
 
 # -*- coding: UTF-8 -*-
+import os
+import tempfile
 import unittest
 
 import numpy as np
@@ -64,5 +66,104 @@ def test_allocation_plan_works_with_only_execute_path_to_fetches_option(self):
             assert_allclose(session_run_results[0], -(inp0 - inp1))
 
 
+class TestBackendKwargsAllowlist(unittest.TestCase):
+    """Tests that the SessionOptions/RunOptions kwargs allowlist correctly blocks
+    dangerous attributes and allows safe ones, preventing arbitrary file writes
+    through user-controlled kwargs."""
+
+    def test_blocked_session_option_optimized_model_filepath_raises(self):
+        """optimized_model_filepath is a known SessionOptions attr but is not in the allowlist.
+        It must raise RuntimeError to prevent arbitrary file overwrites."""
+        name = get_name("mul_1.onnx")
+        with tempfile.NamedTemporaryFile(suffix=".bin") as tmp:
+            with self.assertRaises(RuntimeError) as ctx:
+                backend.prepare(name, optimized_model_filepath=tmp.name)
+            self.assertIn("not permitted", str(ctx.exception))
+
+    def test_blocked_session_option_profile_file_prefix_raises(self):
+        """profile_file_prefix is a known SessionOptions attr but is not in the allowlist.
+        It must raise RuntimeError to prevent arbitrary file writes via profiling output."""
+        name = get_name("mul_1.onnx")
+        with tempfile.TemporaryDirectory() as tmpdir:
+            prefix = os.path.join(tmpdir, "profile")
+            with self.assertRaises(RuntimeError) as ctx:
+                backend.prepare(name, profile_file_prefix=prefix)
+            self.assertIn("not permitted", str(ctx.exception))
+
+    def test_blocked_session_option_enable_profiling_raises(self):
+        """enable_profiling is excluded from the allowlist because it causes uncontrolled
+        file writes (profiling JSON) to the current working directory."""
+        name = get_name("mul_1.onnx")
+        with self.assertRaises(RuntimeError) as ctx:
+            backend.prepare(name, enable_profiling=True)
+        self.assertIn("not permitted", str(ctx.exception))
+
+    def test_unknown_kwarg_is_silently_ignored(self):
+        """A kwarg that is not a SessionOptions attribute at all must be silently ignored.
+        This preserves backward compatibility for callers who pass extra kwargs."""
+        name = get_name("mul_1.onnx")
+        rep = backend.prepare(name, totally_unknown_kwarg="foo")
+        self.assertIsNotNone(rep)
+        x = np.array([[1.0, 2.0], [3.0, 4.0], [5.0, 6.0]], dtype=np.float32)
+        res = rep.run(x)
+        output_expected = np.array([[1.0, 4.0], [9.0, 16.0], [25.0, 36.0]], dtype=np.float32)
+        np.testing.assert_allclose(res[0], output_expected, rtol=1e-05, atol=1e-08)
+
+    def test_safe_session_option_graph_optimization_level_is_accepted(self):
+        """graph_optimization_level is in the allowlist and must be accepted without error."""
+        name = get_name("mul_1.onnx")
+        rep = backend.prepare(name, graph_optimization_level=onnxrt.GraphOptimizationLevel.ORT_DISABLE_ALL)
+        self.assertIsNotNone(rep)
+        x = np.array([[1.0, 2.0], [3.0, 4.0], [5.0, 6.0]], dtype=np.float32)
+        res = rep.run(x)
+        output_expected = np.array([[1.0, 4.0], [9.0, 16.0], [25.0, 36.0]], dtype=np.float32)
+        np.testing.assert_allclose(res[0], output_expected, rtol=1e-05, atol=1e-08)
+
+    def test_safe_session_option_intra_op_num_threads_is_accepted(self):
+        """intra_op_num_threads is in the allowlist and must be accepted without error."""
+        name = get_name("mul_1.onnx")
+        rep = backend.prepare(name, intra_op_num_threads=1)
+        self.assertIsNotNone(rep)
+        x = np.array([[1.0, 2.0], [3.0, 4.0], [5.0, 6.0]], dtype=np.float32)
+        res = rep.run(x)
+        output_expected = np.array([[1.0, 4.0], [9.0, 16.0], [25.0, 36.0]], dtype=np.float32)
+        np.testing.assert_allclose(res[0], output_expected, rtol=1e-05, atol=1e-08)
+
+    def test_blocked_run_option_terminate_raises(self):
+        """terminate is a known RunOptions attr but is not in _ALLOWED_RUN_OPTIONS.
+        Since run_model() now splits kwargs, terminate arrives only in the RunOptions path
+        and must raise RuntimeError."""
+        name = get_name("mul_1.onnx")
+        rep = backend.prepare(name)
+        x = np.array([[1.0, 2.0], [3.0, 4.0], [5.0, 6.0]], dtype=np.float32)
+        with self.assertRaises(RuntimeError) as ctx:
+            rep.run(x, terminate=True)
+        self.assertIn("not permitted", str(ctx.exception))
+
+    def test_run_model_with_safe_session_option(self):
+        """run_model() must accept safe SessionOptions kwargs and produce correct output."""
+        name = get_name("mul_1.onnx")
+        x = np.array([[1.0, 2.0], [3.0, 4.0], [5.0, 6.0]], dtype=np.float32)
+        res = backend.run_model(name, [x], graph_optimization_level=onnxrt.GraphOptimizationLevel.ORT_DISABLE_ALL)
+        output_expected = np.array([[1.0, 4.0], [9.0, 16.0], [25.0, 36.0]], dtype=np.float32)
+        np.testing.assert_allclose(res[0], output_expected, rtol=1e-05, atol=1e-08)
+
+    def test_run_model_with_safe_run_option(self):
+        """run_model() must accept safe RunOptions kwargs and produce correct output."""
+        name = get_name("mul_1.onnx")
+        x = np.array([[1.0, 2.0], [3.0, 4.0], [5.0, 6.0]], dtype=np.float32)
+        res = backend.run_model(name, [x], only_execute_path_to_fetches=True)
+        output_expected = np.array([[1.0, 4.0], [9.0, 16.0], [25.0, 36.0]], dtype=np.float32)
+        np.testing.assert_allclose(res[0], output_expected, rtol=1e-05, atol=1e-08)
+
+    def test_run_model_with_blocked_run_option_raises(self):
+        """run_model() must raise RuntimeError when a blocked RunOptions attr is passed.
+        With kwargs split, terminate arrives only in the RunOptions path and raises."""
+        name = get_name("mul_1.onnx")
+        x = np.array([[1.0, 2.0], [3.0, 4.0], [5.0, 6.0]], dtype=np.float32)
+        with self.assertRaises(RuntimeError):
+            backend.run_model(name, [x], terminate=True)
+
+
 if __name__ == "__main__":
     unittest.main(module=__name__, buffer=True)