Skip to content

Commit 871f11d

Browse files
committed
fix(security): harden untrusted serialized inputs
Signed-off-by: yaoyu-33 <yaoyu.094@gmail.com>
1 parent dd150c1 commit 871f11d

9 files changed

Lines changed: 325 additions & 3 deletions

File tree

src/megatron/bridge/data/energon/base_energon_datamodule.py

Lines changed: 42 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,14 +13,55 @@
1313
# limitations under the License.
1414

1515
import logging
16-
from typing import Any, Literal, Optional
16+
from typing import Any, Callable, Literal, Optional
1717

1818
from megatron.core.process_groups_config import ProcessGroupCollection
1919
from megatron.energon import WorkerConfig, get_savable_loader, get_train_dataset
20+
from megatron.energon.epathlib import EPath
21+
from megatron.energon.flavors.webdataset.default_generic_webdataset import DefaultGenericWebdatasetFactory
2022

2123

2224
logger = logging.getLogger(__name__)
2325

26+
_energon_factory_init = DefaultGenericWebdatasetFactory.__init__
27+
28+
29+
def _secure_energon_factory_init(
30+
self: DefaultGenericWebdatasetFactory,
31+
path: EPath,
32+
*,
33+
subflavors: dict[str, Any] | None = None,
34+
field_map: dict[str, str] | None = None,
35+
sample_loader: str | Callable[[dict[str, Any]], dict[str, Any]] | None = None,
36+
part_filter: str | list[str] | Callable[[str], bool] | None = None,
37+
**kwargs: Any,
38+
) -> None:
39+
"""Reject dataset-local Python hooks before Energon resolves their files."""
40+
executable_fields = [
41+
name
42+
for name, value in (("sample_loader", sample_loader), ("part_filter", part_filter))
43+
if isinstance(value, str)
44+
]
45+
if executable_fields:
46+
raise ValueError(
47+
"Energon dataset metadata cannot load Python files through "
48+
f"{', '.join(executable_fields)}. Use a declarative field_map instead."
49+
)
50+
_energon_factory_init(
51+
self,
52+
path,
53+
subflavors=subflavors,
54+
field_map=field_map,
55+
sample_loader=sample_loader,
56+
part_filter=part_filter,
57+
**kwargs,
58+
)
59+
60+
61+
# Energon constructs this factory internally after reading dataset.yaml, so
62+
# Bridge must install the guard before calling get_train_dataset().
63+
DefaultGenericWebdatasetFactory.__init__ = _secure_energon_factory_init
64+
2465

2566
class EnergonMultiModalDataModule:
2667
"""

src/megatron/bridge/diffusion/data/common/diffusion_task_encoder_with_sp.py

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
from megatron.bridge.data.energon.metadata import sample_metadata_kwargs
2525
from megatron.bridge.data.packing.algorithms import first_fit_decreasing
2626
from megatron.bridge.diffusion.data.common.diffusion_sample import DiffusionSample
27+
from megatron.bridge.diffusion.data.common.safe_decoder import SafeDiffusionSampleDecoder
2728
from megatron.bridge.diffusion.data.common.sequence_packing_utils import packing_length
2829

2930

@@ -51,6 +52,8 @@ def cook(sample: dict) -> dict:
5152

5253

5354
class DiffusionTaskEncoderWithSequencePacking(DefaultTaskEncoder, ABC): # noqa: D101
55+
decoder = SafeDiffusionSampleDecoder()
56+
5457
cookers = [
5558
Cooker(cook),
5659
]
Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,52 @@
1+
# Copyright (c) 2026, NVIDIA CORPORATION. All rights reserved.
2+
#
3+
# Licensed under the Apache License, Version 2.0 (the "License");
4+
# you may not use this file except in compliance with the License.
5+
# You may obtain a copy of the License at
6+
#
7+
# http://www.apache.org/licenses/LICENSE-2.0
8+
#
9+
# Unless required by applicable law or agreed to in writing, software
10+
# distributed under the License is distributed on an "AS IS" BASIS,
11+
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
# See the License for the specific language governing permissions and
13+
# limitations under the License.
14+
15+
"""Safe decoding for Bridge diffusion WebDataset fields."""
16+
17+
import io
18+
import re
19+
from typing import Any
20+
21+
import torch
22+
from megatron.energon.flavors.webdataset.sample_decoder import SampleDecoder
23+
from webdataset.autodecode import basichandlers
24+
25+
from megatron.bridge.utils.safe_pickle import safe_torch_tensor_pickle_loads
26+
27+
28+
_PICKLE_EXTENSIONS = frozenset({"pickle", "pkl", "pyd"})
29+
30+
31+
def _safe_basic_handler(key: str, data: bytes) -> Any:
32+
"""Decode tensor fields safely and delegate non-executable formats."""
33+
extension = re.sub(r".*[.]", "", key).lower()
34+
if extension in _PICKLE_EXTENSIONS:
35+
return safe_torch_tensor_pickle_loads(data)
36+
if extension == "pth":
37+
return torch.load(io.BytesIO(data), map_location="cpu", weights_only=True)
38+
return basichandlers(key, data)
39+
40+
41+
class SafeDiffusionSampleDecoder(SampleDecoder):
42+
"""Energon decoder that never uses WebDataset's unrestricted pickle handlers."""
43+
44+
def __init__(self) -> None:
45+
super().__init__()
46+
replaced = False
47+
for index, handler in enumerate(self._decoder.handlers):
48+
if handler is basichandlers:
49+
self._decoder.handlers[index] = _safe_basic_handler
50+
replaced = True
51+
if not replaced:
52+
raise RuntimeError("Energon SampleDecoder no longer exposes the expected WebDataset basic handler.")

src/megatron/bridge/utils/instantiate_utils.py

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,15 @@
105105
},
106106
}
107107

108+
# Resolve aliases before enforcing these entries. For example,
109+
# ``numpy.lib.npyio.load`` is the same callable as ``numpy.load`` and
110+
# ``torch.serialization.load`` is the same callable as ``torch.load``.
111+
_DISALLOWED_CANONICAL_TARGETS: set[str] = {
112+
"numpy.load",
113+
"torch.serialization.load",
114+
"transformers.dynamic_module_utils.get_class_from_dynamic_module",
115+
}
116+
108117
_DISALLOWED_CALLABLE_FIELD_NAMES: set[str] = {
109118
"collate_impl",
110119
"hf_filter_lambda",
@@ -182,7 +191,17 @@ def _resolve_target(
182191
"""Resolve target string, type, or callable after Bridge validation."""
183192
if isinstance(target, str):
184193
_validate_target_prefix(target=target, full_key=full_key)
185-
return _mcore_resolve_target(target, full_key, check_callable)
194+
resolved_target = _mcore_resolve_target(target, full_key, check_callable)
195+
if isinstance(target, str):
196+
module = getattr(resolved_target, "__module__", None)
197+
qualname = getattr(resolved_target, "__qualname__", None)
198+
canonical_target = f"{module}.{qualname}" if isinstance(module, str) and isinstance(qualname, str) else None
199+
if canonical_target in _DISALLOWED_CANONICAL_TARGETS:
200+
raise InstantiationException(
201+
f"Instantiation of '{target}' is not allowed because it resolves to the unsafe target "
202+
f"'{canonical_target}'." + (f"\nfull_key: {full_key}" if full_key else "")
203+
)
204+
return resolved_target
186205

187206

188207
_mcore_instantiate_utils._resolve_target = _resolve_target

src/megatron/bridge/utils/safe_pickle.py

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -239,6 +239,37 @@ def find_class(self, module: str, name: str) -> object:
239239
)
240240

241241

242+
def _safe_torch_load_from_bytes(data: bytes) -> object:
243+
"""Reconstruct tensor storage bytes without enabling arbitrary pickle globals."""
244+
import torch
245+
246+
return torch.load(io.BytesIO(data), map_location="cpu", weights_only=True)
247+
248+
249+
class _TorchTensorRestrictedUnpickler(_RestrictedUnpickler):
250+
"""Restricted unpickler for plain tensors and containers of tensors."""
251+
252+
_SAFE_MODULES = MappingProxyType(
253+
{
254+
**_RestrictedUnpickler._SAFE_MODULES,
255+
"torch._utils": frozenset({"_rebuild_tensor", "_rebuild_tensor_v2"}),
256+
}
257+
)
258+
259+
def find_class(self, module: str, name: str) -> object:
260+
if (module, name) in {
261+
("megatron.core.safe_globals", "safe_load_from_bytes"),
262+
("torch.storage", "_load_from_bytes"),
263+
}:
264+
return _safe_torch_load_from_bytes
265+
if module in self._SAFE_MODULES and name in self._SAFE_MODULES[module]:
266+
return pickle.Unpickler.find_class(self, module, name)
267+
raise pickle.UnpicklingError(
268+
f"Restricted unpickler refused to load '{module}.{name}'. "
269+
"Only safe built-in types and plain torch tensors are allowed."
270+
)
271+
272+
242273
class _NumpyRestrictedUnpickler(pickle.Unpickler):
243274
"""Unpickler that allows safe builtins and the narrow set of numpy types needed for object array reconstruction.
244275
@@ -428,6 +459,11 @@ def safe_pickle_loads(data: bytes) -> object:
428459
return _RestrictedUnpickler(io.BytesIO(data)).load()
429460

430461

462+
def safe_torch_tensor_pickle_loads(data: bytes) -> object:
463+
"""Deserialize raw pickle data containing only safe containers and plain torch tensors."""
464+
return _TorchTensorRestrictedUnpickler(io.BytesIO(data)).load()
465+
466+
431467
def safe_load_npy(data: bytes):
432468
"""Load a ``.npy`` file from raw bytes without enabling unrestricted pickle.
433469
Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
import pytest
2+
from megatron.energon.dataset_config import load_config
3+
from megatron.energon.epathlib import EPath
4+
from megatron.energon.flavors.webdataset.default_generic_webdataset import DefaultGenericWebdatasetFactory
5+
6+
from megatron.bridge.data.energon import base_energon_datamodule
7+
8+
9+
@pytest.mark.parametrize("field", ["sample_loader", "part_filter"])
10+
def test_dataset_yaml_rejects_python_hooks_before_import(field, tmp_path):
11+
marker = tmp_path / "dataset-python-executed"
12+
metadata_dir = tmp_path / ".nv-meta"
13+
metadata_dir.mkdir()
14+
module_path = metadata_dir / "evil.py"
15+
module_path.write_text(f"from pathlib import Path\nPath({str(marker)!r}).touch()\n")
16+
config_path = metadata_dir / "dataset.yaml"
17+
config_path.write_text(
18+
"sample_loader: evil.py\npart_filter:\n - json\n"
19+
if field == "sample_loader"
20+
else "part_filter: evil.py\n"
21+
)
22+
default_kwargs = {"path": EPath(tmp_path)}
23+
if field == "part_filter":
24+
default_kwargs["sample_loader"] = lambda sample: sample
25+
26+
with pytest.raises(ValueError, match="cannot load Python files"):
27+
load_config(
28+
EPath(config_path),
29+
default_type=DefaultGenericWebdatasetFactory,
30+
default_kwargs=default_kwargs,
31+
)
32+
33+
assert not marker.exists()
34+
35+
36+
def test_factory_guard_preserves_callable_hooks(monkeypatch, tmp_path):
37+
captured = {}
38+
39+
def original_init(self, path, **kwargs):
40+
captured.update(kwargs)
41+
42+
monkeypatch.setattr(base_energon_datamodule, "_energon_factory_init", original_init)
43+
44+
def sample_loader(sample):
45+
return sample
46+
47+
def part_filter(_part):
48+
return True
49+
50+
base_energon_datamodule._secure_energon_factory_init(
51+
object(), EPath(tmp_path), sample_loader=sample_loader, part_filter=part_filter
52+
)
53+
54+
assert captured["sample_loader"] is sample_loader
55+
assert captured["part_filter"] is part_filter
Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,59 @@
1+
import io
2+
import os
3+
import pickle
4+
5+
import pytest
6+
import torch
7+
from webdataset.autodecode import DecodingError
8+
9+
from megatron.bridge.diffusion.data.common.safe_decoder import SafeDiffusionSampleDecoder
10+
11+
12+
class _WriteMarkerPayload:
13+
def __init__(self, marker: str) -> None:
14+
self.marker = marker
15+
16+
def __reduce__(self):
17+
return os.system, (f"touch {self.marker}",)
18+
19+
20+
@pytest.mark.parametrize("extension", ["pickle", "pkl", "pyd"])
21+
def test_safe_decoder_loads_plain_tensor_pickles(extension):
22+
decoder = SafeDiffusionSampleDecoder()
23+
value = {"embedding": torch.arange(4, dtype=torch.float32), "shape": [1, 4]}
24+
25+
restored = decoder.decode(f"sample.{extension}", pickle.dumps(value))
26+
27+
assert restored["shape"] == value["shape"]
28+
assert torch.equal(restored["embedding"], value["embedding"])
29+
30+
31+
def test_safe_decoder_loads_weights_only_pth():
32+
decoder = SafeDiffusionSampleDecoder()
33+
value = {"embedding": torch.arange(4, dtype=torch.float32), "shape": [1, 4]}
34+
buffer = io.BytesIO()
35+
torch.save(value, buffer)
36+
37+
restored = decoder.decode("sample.pth", buffer.getvalue())
38+
39+
assert restored["shape"] == value["shape"]
40+
assert torch.equal(restored["embedding"], value["embedding"])
41+
42+
43+
@pytest.mark.parametrize("extension", ["pickle", "pth"])
44+
def test_safe_decoder_rejects_executable_payloads(extension, tmp_path):
45+
decoder = SafeDiffusionSampleDecoder()
46+
marker = tmp_path / f"{extension}-executed"
47+
payload = _WriteMarkerPayload(str(marker))
48+
if extension == "pickle":
49+
data = pickle.dumps(payload)
50+
else:
51+
buffer = io.BytesIO()
52+
torch.save(payload, buffer)
53+
data = buffer.getvalue()
54+
55+
with pytest.raises(DecodingError) as error:
56+
decoder.decode(f"sample.{extension}", data)
57+
58+
assert isinstance(error.value.__cause__, pickle.UnpicklingError)
59+
assert not marker.exists()

tests/unit_tests/utils/test_instantiate_utils.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -810,6 +810,29 @@ def test_instantiate_rejects_deserialization_and_native_loader_targets(self, tar
810810
with pytest.raises(InstantiationException, match="bypass target validation"):
811811
instantiate(config)
812812

813+
@pytest.mark.parametrize(
814+
"target",
815+
[
816+
"numpy.lib.npyio.load",
817+
"torch.serialization.load",
818+
"transformers.dynamic_module_utils.get_class_from_dynamic_module",
819+
],
820+
)
821+
def test_instantiate_rejects_canonical_aliases_and_dynamic_code_helpers(self, target):
822+
"""Test that aliases and dynamic-code helpers cannot bypass exact target checks."""
823+
with pytest.raises(InstantiationException, match="resolves to the unsafe target"):
824+
instantiate({"_target_": target, "_call_": False})
825+
826+
def test_instantiate_rejects_unsafe_alias_hidden_in_unused_kwarg(self):
827+
"""Test that recursive instantiation rejects unsafe targets before filtering kwargs."""
828+
config = {
829+
"_target_": "tests.unit_tests.utils.test_instantiate_utils.TestClass",
830+
"name": "safe",
831+
"unused": {"_target_": "numpy.lib.npyio.load", "_call_": False},
832+
}
833+
with pytest.raises(InstantiationException, match="resolves to the unsafe target"):
834+
instantiate(config)
835+
813836
@pytest.mark.parametrize(
814837
"target",
815838
[

0 commit comments

Comments
 (0)