fix: restrict algo_from_json _target_ to Algo subclasses (GHSA-2wx3)

monai/auto3dseg/utils.py

@@ -20,6 +20,7 @@
 import warnings
 from copy import deepcopy
 from numbers import Number
+from pydoc import locate
 from typing import Any, cast
 
 import numpy as np
@@ -62,6 +63,42 @@ def _require_pickle_allowed() -> None:
         raise RuntimeError(_PICKLE_DISABLED_MSG)
 
 
+def _reject_non_algo_target(target: object, filename: str) -> None:
+    """
+    Require that ``target`` names an ``Algo`` subclass before it is instantiated.
+
+    ``algo_from_json`` resolves ``_target_`` through ``ConfigParser``, which imports the dotted path
+    and calls it. ``algo_object.json`` has exactly one legitimate target type -- an ``Algo``
+    subclass -- so resolving the name and checking it here rejects payloads such as
+    ``subprocess.call`` or ``os.system`` before they are ever invoked.
+
+    Importing the named module still runs that module's top-level code, so this narrows the sink
+    rather than closing it: an untrusted ``algo_object.json`` combined with an attacker-controlled
+    template directory remains dangerous.
+
+    Args:
+        target: the ``_target_`` value read from the file.
+        filename: the file the value came from, used in the error message.
+
+    Raises:
+        ValueError: if ``target`` is not a string, or does not resolve to a concrete ``Algo`` subclass.
+        ModuleNotFoundError: if the module cannot be imported, so callers can try the next
+            template path.
+    """
+    if not isinstance(target, str):
+        raise ValueError(f"invalid `_target_` in {filename}: expected a string, got {type(target).__name__}.")
+    resolved = locate(target)
+    if resolved is None:
+        raise ModuleNotFoundError(f"cannot resolve `_target_` {target!r} from {filename}.")
+    if not (isinstance(resolved, type) and resolved is not Algo and issubclass(resolved, Algo)):
+        raise ValueError(
+            f"refusing to instantiate `_target_` {target!r} from {filename}: it resolves to "
+            f"{resolved!r}, which is not a subclass of monai.auto3dseg.Algo. Only Algo subclasses "
+            "may be named in an algo_object.json "
+            "(see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-2wx3-8x3w-r8qv)."
+        )
+
+
 measure_np, has_measure = optional_import("skimage.measure", "0.14.2", min_version)
 cp, has_cp = optional_import("cupy")
 
@@ -493,16 +530,16 @@ def algo_from_json(filename: str, template_path: PathLike | None = None, **kwarg
             if state_template_path:
                 algo_config["template_path"] = state_template_path
 
-            warnings.warn(
-                f"Loading {filename}: the file's `_target_` value is resolved to an imported callable and "
-                "invoked, and template directories from the file may be added to `sys.path`; only load "
-                "algo_object.json files from a source you trust "
-                "(see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-2wx3-8x3w-r8qv).",
-                stacklevel=2,
-            )
+            _reject_non_algo_target(target, filename)
 
             parser = ConfigParser(algo_config)
             algo = parser.get_parsed_content()
+            if not isinstance(algo, Algo):
+                raise ValueError(
+                    f"refusing to return the object built from '{target}' in {filename}: it is a "
+                    f"{type(algo).__name__}, not an Algo instance "
+                    "(see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-2wx3-8x3w-r8qv)."
+                )
             used_template_path = path
             break
         except ModuleNotFoundError as e:

tests/apps/test_auto3dseg.py

@@ -15,7 +15,6 @@
 import os
 import tempfile
 import unittest
-import warnings
 from copy import deepcopy
 from numbers import Number
 
@@ -637,22 +636,20 @@ def tearDown(self) -> None:
 
 
 class TestAlgoFromJsonSecurityWarning(unittest.TestCase):
-    def test_warns_about_untrusted_target(self) -> None:
+    def test_rejects_untrusted_target(self) -> None:
+        """Verify that a ``_target_`` pointing to a non-``Algo`` class is rejected.
+
+        ``_DummyAlgo`` is a plain class (not an ``Algo`` subclass); loading it must raise
+        ``ValueError`` rather than instantiate the class, per GHSA-2wx3.
+        """
         with tempfile.TemporaryDirectory() as tmpdir:
             algo_file = os.path.join(tmpdir, "algo_object.json")
             with open(algo_file, "w", encoding="utf-8") as f:
                 json.dump({"_target_": f"{__name__}._DummyAlgo"}, f)
 
-            with warnings.catch_warnings(record=True) as caught:
-                warnings.simplefilter("always")
+            with self.assertRaisesRegex(ValueError, "refusing to instantiate"):
                 algo_from_json(algo_file)
 
-            messages = [str(w.message) for w in caught]
-            self.assertTrue(
-                any("algo_object.json" in msg and "trust" in msg for msg in messages),
-                f"Keywords 'algo_object.json' and 'trust' not found in warning messages: {messages}",
-            )
-
 
 if __name__ == "__main__":
     unittest.main()

tests/auto3dseg/test_algo_target_allowlist.py

@@ -0,0 +1,75 @@
+# Copyright (c) MONAI Consortium
+# Licensed under the Apache License, Version 2.0 (the "License");
+# you may not use this file except in compliance with the License.
+# You may obtain a copy of the License at
+#     http://www.apache.org/licenses/LICENSE-2.0
+# Unless required by applicable law or agreed to in writing, software
+# distributed under the License is distributed on an "AS IS" BASIS,
+# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+# See the License for the specific language governing permissions and
+# limitations under the License.
+
+from __future__ import annotations
+
+import json
+import os
+import tempfile
+import unittest
+
+from monai.auto3dseg.utils import _reject_non_algo_target, algo_from_json
+
+
+class TestAlgoTargetAllowlist(unittest.TestCase):
+    """Regression tests for GHSA-2wx3-8x3w-r8qv.
+
+    ``algo_from_json`` resolves the file's ``_target_`` through ``ConfigParser``, which imports the
+    dotted path and calls it. ``algo_object.json`` has exactly one legitimate target type, so a
+    ``_target_`` that is not an ``Algo`` subclass is rejected before it is instantiated.
+    """
+
+    def test_code_execution_targets_are_rejected(self):
+        for target in ("subprocess.call", "os.system", "builtins.eval", "builtins.exec", "shutil.rmtree"):
+            with self.subTest(target=target):
+                with self.assertRaises(ValueError) as ctx:
+                    _reject_non_algo_target(target, "algo_object.json")
+                self.assertIn("GHSA-2wx3-8x3w-r8qv", str(ctx.exception))
+
+    def test_non_class_target_is_rejected(self):
+        """A module or plain function is not an Algo subclass."""
+        with self.assertRaisesRegex(ValueError, r"GHSA-2wx3-8x3w-r8qv"):
+            _reject_non_algo_target("json.dumps", "algo_object.json")
+
+    def test_unresolvable_target_raises_module_not_found(self):
+        """Unresolvable names raise ModuleNotFoundError so the caller can try the next path."""
+        with self.assertRaises(ModuleNotFoundError):
+            _reject_non_algo_target("no_such_module.NoSuchAlgo", "algo_object.json")
+
+    def test_algo_subclass_is_accepted(self):
+        """A real Algo subclass passes the check."""
+        _reject_non_algo_target("monai.apps.auto3dseg.BundleAlgo", "algo_object.json")
+
+    def test_non_string_target_is_rejected(self):
+        """A JSON ``null`` ``_target_`` raises the documented ``ValueError``, not ``AttributeError``."""
+        with self.assertRaisesRegex(ValueError, r"expected a string"):
+            _reject_non_algo_target(None, "algo_object.json")
+
+    def test_bare_algo_base_class_is_rejected(self):
+        """The abstract ``Algo`` base class is not itself a valid instantiation target."""
+        with self.assertRaisesRegex(ValueError, r"GHSA-2wx3-8x3w-r8qv"):
+            _reject_non_algo_target("monai.auto3dseg.algo_gen.Algo", "algo_object.json")
+
+    def test_algo_from_json_rejects_payload_target(self):
+        """End to end: a malicious algo_object.json never reaches instantiation."""
+        with tempfile.TemporaryDirectory() as tempdir:
+            path = os.path.join(tempdir, "algo_object.json")
+            marker = os.path.join(tempdir, "PWNED")
+            with open(path, "w") as f:
+                json.dump({"_target_": "subprocess.call", "args": ["/bin/sh", "-c", f"touch {marker}"]}, f)
+
+            with self.assertRaisesRegex(ValueError, r"GHSA-2wx3-8x3w-r8qv"):
+                algo_from_json(path)
+            self.assertFalse(os.path.exists(marker), "the algo_object.json payload executed")
+
+
+if __name__ == "__main__":
+    unittest.main()