security: replace unrestricted setattr with allowlist in Python backend

.github/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

@@ -17,6 +17,25 @@
 from onnxruntime import InferenceSession, SessionOptions, get_available_providers, get_device
 from onnxruntime.backend.backend_rep import OnnxRuntimeBackendRep
 
+# 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 +120,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 +131,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: {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,7 +179,8 @@ 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: only a safe subset of :class:`onnxruntime.RunOptions` attributes are
+            accepted; see ``_ALLOWED_RUN_OPTIONS`` in ``backend_rep.py`` for the list
         :return: predictions
         """
         rep = cls.prepare(model, device, **kwargs)

onnxruntime/python/backend/backend_rep.py

@@ -10,6 +10,17 @@
 
 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.
+# SessionOptions keys forwarded from run_model() are silently ignored here.
+_ALLOWED_RUN_OPTIONS = frozenset({
+    "log_severity_level",
+    "log_verbosity_level",
+    "logid",
+    "only_execute_path_to_fetches",
+})
+
 
 class OnnxRuntimeBackendRep(BackendRep):
     """
@@ -31,8 +42,10 @@ 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)
+            # Unknown keys are silently ignored: run_model() forwards the same kwargs
+            # used for SessionOptions, so those keys will arrive here and must not raise.
 
         if isinstance(inputs, list):
             inps = {}

onnxruntime/test/python/onnxruntime_test_python_backend.py

@@ -64,5 +64,73 @@ 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 self.assertRaises(RuntimeError):
+            backend.prepare(name, optimized_model_filepath="/tmp/should_not_exist.bin")
+
+    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 self.assertRaises(RuntimeError):
+            backend.prepare(name, profile_file_prefix="/tmp/should_not_exist_profile")
+
+    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):
+            backend.prepare(name, enable_profiling=True)
+
+    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_is_silently_ignored(self):
+        """terminate is a known RunOptions attr but is not in _ALLOWED_RUN_OPTIONS.
+        run_model() forwards the same kwargs to both prepare() and rep.run(), so blocked
+        RunOptions attrs must be silently ignored — not raise — to avoid breaking run_model()."""
+        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)
+        res = rep.run(x, terminate=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)
+
+
 if __name__ == "__main__":
     unittest.main(module=__name__, buffer=True)