Skip to content

Commit 95c1dce

Browse files
garciadiasclaude
andcommitted
Fix GHSA-873f-pvrv-4x83: warn before executing a bundle's config
monai.bundle.load(), with its default model=None, builds a bundle's network by parsing the bundle's own config through create_workflow(). That parsing resolves any "_target_" value to an importable callable with no allow list, and passes any "$"-prefixed value to Python eval(). monai.bundle.run() reaches the same path via a caller-supplied config_file. Either way, loading or running a bundle whose config hasn't been reviewed can execute arbitrary code. create_workflow() -- the shared path both load() and run() use to parse a config file -- now raises a UserWarning immediately before doing so, describing what "_target_"/"$"-expression content can do and linking the advisory. This applies uniformly to every caller of create_workflow(), not just load(). No behavior is blocked: the config is still parsed and executed exactly as before, just with a warning first. An earlier version of this fix added an opt-in trust_remote_code flag to load(), but that was dropped after review: MONAI has no way to establish whether a bundle is actually trustworthy, so a flag like that would only teach callers to set it once and forget about it. Update docstrings on load(), run(), and create_workflow() to describe the risk and point at the advisory. Add TestLoadWarnsOnConfigExecution to tests/bundle/test_bundle_download.py: default load() warns and still executes the config, explicit model= skips config parsing entirely and warns about nothing, and run() warns via the same create_workflow() path. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Signed-off-by: R. Garcia-Dias <rafaelagd@gmail.com>
1 parent 87060c4 commit 95c1dce

2 files changed

Lines changed: 93 additions & 3 deletions

File tree

monai/bundle/scripts.py

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -648,6 +648,14 @@ def load(
648648
"""
649649
Load model weights or TorchScript module of a bundle.
650650
651+
Security note: if `model` is `None`, building `network_def` requires parsing the bundle's own
652+
"{workflow_type}.json" config, which can define `"_target_"` components resolved to any importable
653+
callable and `"$"`-prefixed expressions evaluated with Python `eval()`. Only call `load()` this way
654+
for bundles from a source you trust; a warning is printed every time this happens
655+
(see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-873f-pvrv-4x83). To skip parsing
656+
the bundle's config entirely, pass an explicit `model=` — only the weights are then loaded, via
657+
`torch.load(..., weights_only=True)`.
658+
651659
Args:
652660
name: bundle name. If `None` and `url` is `None`, it must be provided in `args_file`.
653661
for example:
@@ -935,6 +943,12 @@ def run(
935943
"""
936944
Specify `config_file` to run monai bundle components and workflows.
937945
946+
Security note: parsing `config_file` can run arbitrary code. Any `"_target_"` value is resolved to an
947+
importable callable and invoked with no allow list, and any `"$"`-prefixed value is passed to Python
948+
`eval()`. Only point this at config files you wrote or otherwise fully trust; never at a config
949+
downloaded from, or otherwise sourced from, an untrusted party. A warning is printed every time this
950+
happens (see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-873f-pvrv-4x83).
951+
938952
Typical usage examples:
939953
940954
.. code-block:: bash
@@ -1929,6 +1943,12 @@ def create_workflow(
19291943
The workflow should be subclass of `BundleWorkflow` and be available to import.
19301944
It can be MONAI existing bundle workflows or user customized workflows.
19311945
1946+
Security note: parsing `config_file` can run arbitrary code. Any `"_target_"` value is resolved to an
1947+
importable callable and invoked with no allow list, and any `"$"`-prefixed value is passed to Python
1948+
`eval()`. Only point this at config files you wrote or otherwise fully trust; never at a config
1949+
downloaded from, or otherwise sourced from, an untrusted party. A warning is printed every time this
1950+
happens (see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-873f-pvrv-4x83).
1951+
19321952
Typical usage examples:
19331953
19341954
.. code-block:: python
@@ -1966,6 +1986,13 @@ def create_workflow(
19661986
)
19671987

19681988
if config_file is not None:
1989+
warnings.warn(
1990+
f"parsing config_file {config_file}: any `\"_target_\"` value in it is resolved to an importable "
1991+
"callable and invoked with no allow list, and any `\"$\"`-prefixed value is passed to Python "
1992+
"`eval()`. Only proceed if this config is from a source you trust "
1993+
"(see https://github.com/Project-MONAI/MONAI/security/advisories/GHSA-873f-pvrv-4x83).",
1994+
stacklevel=2,
1995+
)
19691996
workflow_ = workflow_class(config_file=config_file, **_args)
19701997
else:
19711998
workflow_ = workflow_class(**_args)

tests/bundle/test_bundle_download.py

Lines changed: 66 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@
2424

2525
import monai.networks.nets as nets
2626
from monai.apps import check_hash
27-
from monai.bundle import ConfigParser, create_workflow, load
27+
from monai.bundle import ConfigParser, create_workflow, load, run
2828
from monai.bundle.scripts import _examine_monai_version, _list_latest_versions, download
2929
from monai.utils import optional_import
3030
from tests.test_utils import (
@@ -368,7 +368,13 @@ def test_load_weights_with_net_override(self, bundle_name, device, net_override)
368368
# download bundle, and load weights from the downloaded path
369369
with tempfile.TemporaryDirectory() as tempdir:
370370
# load weights
371-
model = load(name=bundle_name, bundle_dir=tempdir, source="monaihosting", progress=False, device=device)
371+
model = load(
372+
name=bundle_name,
373+
bundle_dir=tempdir,
374+
source="monaihosting",
375+
progress=False,
376+
device=device,
377+
)
372378

373379
# prepare data and test
374380
input_tensor = torch.rand(1, 1, 96, 96, 96).to(device)
@@ -477,7 +483,11 @@ def test_ngc_download_bundle(self, bundle_name, version, remove_prefix, download
477483
self.assertTrue(check_hash(filepath=full_file_path, val=hash_val))
478484

479485
model = load(
480-
name=bundle_name, source="ngc", version=version, bundle_dir=tempdir, remove_prefix=remove_prefix
486+
name=bundle_name,
487+
source="ngc",
488+
version=version,
489+
bundle_dir=tempdir,
490+
remove_prefix=remove_prefix,
481491
)
482492
assert_allclose(
483493
model.state_dict()[TESTCASE_NGC_WEIGHTS["key"]],
@@ -488,5 +498,58 @@ def test_ngc_download_bundle(self, bundle_name, version, remove_prefix, download
488498
)
489499

490500

501+
class TestLoadWarnsOnConfigExecution(unittest.TestCase):
502+
"""Regression tests for GHSA-873f-pvrv-4x83: `load()`/`create_workflow()` parse and execute a
503+
bundle's own config (arbitrary `_target_`/`$`-expression content) whenever `model` is `None`.
504+
There is no opt-in flag -- MONAI has no way to establish whether a bundle is actually
505+
trustworthy, so a flag would only teach callers to always pass it and ignore the risk. Instead,
506+
a `UserWarning` is raised every time this happens, in both `load()` (via `create_workflow()`)
507+
and `run()` (also via `create_workflow()`)."""
508+
509+
def _stage_malicious_bundle(self, tempdir: str, marker: str) -> str:
510+
name = "evil_bundle"
511+
bundle_root = os.path.join(tempdir, name)
512+
os.makedirs(os.path.join(bundle_root, "configs"))
513+
os.makedirs(os.path.join(bundle_root, "models"))
514+
torch.save({"state_dict": {}}, os.path.join(bundle_root, "models", "model.pt"))
515+
malicious_config = {"network_def": f"$__import__('os').system('echo pwned > {marker}')", "initialize": []}
516+
with open(os.path.join(bundle_root, "configs", "train.json"), "w") as f:
517+
json.dump(malicious_config, f)
518+
return name
519+
520+
def test_default_warns_and_executes_config(self):
521+
with tempfile.TemporaryDirectory() as tempdir:
522+
marker = os.path.join(tempdir, "PWNED")
523+
name = self._stage_malicious_bundle(tempdir, marker)
524+
with self.assertWarns(UserWarning):
525+
with self.assertRaises(AttributeError):
526+
# the malicious config is missing metadata.json and returns a plain `int` for
527+
# `network_def`, so the workflow construction fails after the payload has already
528+
# run -- this mirrors the advisory's own PoC, where the failure happens *after* RCE.
529+
load(name=name, bundle_dir=tempdir, source="github", repo="attacker/repo")
530+
self.assertTrue(os.path.exists(marker))
531+
532+
def test_explicit_model_skips_config_parsing(self):
533+
with tempfile.TemporaryDirectory() as tempdir:
534+
marker = os.path.join(tempdir, "PWNED")
535+
name = self._stage_malicious_bundle(tempdir, marker)
536+
model = nets.UNet(spatial_dims=2, in_channels=1, out_channels=1, channels=(4, 8), strides=(2,))
537+
load(name=name, model=model, bundle_dir=tempdir, source="github", repo="attacker/repo")
538+
self.assertFalse(os.path.exists(marker))
539+
540+
def test_run_warns_on_config_execution(self):
541+
with tempfile.TemporaryDirectory() as tempdir:
542+
marker = os.path.join(tempdir, "PWNED")
543+
config_file = os.path.join(tempdir, "train.json")
544+
with open(config_file, "w") as f:
545+
json.dump({"initialize": [f"$__import__('os').system('echo pwned > {marker}')"]}, f)
546+
with self.assertWarns(UserWarning):
547+
with self.assertRaises(ValueError):
548+
# no "run" ID is defined, so `workflow.run()` fails after `initialize()` has
549+
# already evaluated the payload above.
550+
run(config_file=config_file)
551+
self.assertTrue(os.path.exists(marker))
552+
553+
491554
if __name__ == "__main__":
492555
unittest.main()

0 commit comments

Comments
 (0)