From 1ddca167531fee272aefef8047e3e45d89e70767 Mon Sep 17 00:00:00 2001 From: banteg <4562643+banteg@users.noreply.github.com> Date: Fri, 7 Mar 2025 08:57:08 +0400 Subject: [PATCH 1/2] Remove Loader alias to UnsafeLoader and enhance security MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove Loader class as an alias to UnsafeLoader * Add warning messages when using UnsafeLoader, CUnsafeLoader, unsafe_load(), and unsafe_load_all() * Change default loader in scan(), parse(), compose(), and compose_all() from UnsafeLoader to SafeLoader * Improve documentation with clear security warnings These changes help prevent accidental RCE vulnerabilities when processing untrusted YAML data. 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- CHANGES | 4 +++- lib/yaml/__init__.py | 55 +++++++++++++++++++++++++++----------------- lib/yaml/cyaml.py | 15 ++++++------ lib/yaml/loader.py | 24 ++++++++----------- 4 files changed, 53 insertions(+), 45 deletions(-) diff --git a/CHANGES b/CHANGES index 71a32f44b..b8ce3f837 100644 --- a/CHANGES +++ b/CHANGES @@ -5,7 +5,9 @@ For a complete changelog, see: 7.0.0.dev0 (TBD) -* TBD +* Remove `Loader` class as an alias to `UnsafeLoader` +* Add warning messages when using `UnsafeLoader`, `CUnsafeLoader`, `unsafe_load()`, and `unsafe_load_all()` +* Change default loader in `scan()`, `parse()`, `compose()`, and `compose_all()` from `UnsafeLoader` to `SafeLoader` 6.0.2 (2024-08-06) diff --git a/lib/yaml/__init__.py b/lib/yaml/__init__.py index 971b7f73f..580ecfce2 100644 --- a/lib/yaml/__init__.py +++ b/lib/yaml/__init__.py @@ -5,17 +5,18 @@ from .events import * from .nodes import * -from .loader import * +from .loader import BaseLoader, FullLoader, SafeLoader, UnsafeLoader from .dumper import * __version__ = '7.0.0.dev0' try: - from .cyaml import * + from .cyaml import CBaseLoader, CSafeLoader, CFullLoader, CUnsafeLoader, CBaseDumper, CSafeDumper, CDumper __with_libyaml__ = True except ImportError: __with_libyaml__ = False import io +import warnings #------------------------------------------------------------------------------ # XXX "Warnings control" is now deprecated. Leaving in the API function to not @@ -26,7 +27,7 @@ def warnings(settings=None): return {} #------------------------------------------------------------------------------ -def scan(stream, Loader=Loader): +def scan(stream, Loader=SafeLoader): """ Scan a YAML stream and produce scanning tokens. """ @@ -37,7 +38,7 @@ def scan(stream, Loader=Loader): finally: loader.dispose() -def parse(stream, Loader=Loader): +def parse(stream, Loader=SafeLoader): """ Parse a YAML stream and produce parsing events. """ @@ -48,7 +49,7 @@ def parse(stream, Loader=Loader): finally: loader.dispose() -def compose(stream, Loader=Loader): +def compose(stream, Loader=SafeLoader): """ Parse the first YAML document in a stream and produce the corresponding representation tree. @@ -59,7 +60,7 @@ def compose(stream, Loader=Loader): finally: loader.dispose() -def compose_all(stream, Loader=Loader): +def compose_all(stream, Loader=SafeLoader): """ Parse all YAML documents in a stream and produce corresponding representation trees. @@ -141,7 +142,15 @@ def unsafe_load(stream): Resolve all tags, even those known to be unsafe on untrusted input. - """ + + WARNING: This function is dangerous and can execute arbitrary code. + Only use on trusted input! Use safe_load() or full_load() instead. + """ + warnings.warn( + "unsafe_load() is dangerous and can execute arbitrary code when loading untrusted YAML data. " + "Use safe_load() or full_load() instead.", + RuntimeWarning, stacklevel=2 + ) return load(stream, UnsafeLoader) def unsafe_load_all(stream): @@ -151,7 +160,15 @@ def unsafe_load_all(stream): Resolve all tags, even those known to be unsafe on untrusted input. - """ + + WARNING: This function is dangerous and can execute arbitrary code. + Only use on trusted input! Use safe_load_all() or full_load_all() instead. + """ + warnings.warn( + "unsafe_load_all() is dangerous and can execute arbitrary code when loading untrusted YAML data. " + "Use safe_load_all() or full_load_all() instead.", + RuntimeWarning, stacklevel=2 + ) return load_all(stream, UnsafeLoader) def emit(events, stream=None, Dumper=Dumper, @@ -277,9 +294,8 @@ def add_implicit_resolver(tag, regexp, first=None, first is a sequence of possible initial characters or None. """ if Loader is None: - loader.Loader.add_implicit_resolver(tag, regexp, first) - loader.FullLoader.add_implicit_resolver(tag, regexp, first) - loader.UnsafeLoader.add_implicit_resolver(tag, regexp, first) + FullLoader.add_implicit_resolver(tag, regexp, first) + UnsafeLoader.add_implicit_resolver(tag, regexp, first) else: Loader.add_implicit_resolver(tag, regexp, first) Dumper.add_implicit_resolver(tag, regexp, first) @@ -292,9 +308,8 @@ def add_path_resolver(tag, path, kind=None, Loader=None, Dumper=Dumper): Keys can be string values, integers, or None. """ if Loader is None: - loader.Loader.add_path_resolver(tag, path, kind) - loader.FullLoader.add_path_resolver(tag, path, kind) - loader.UnsafeLoader.add_path_resolver(tag, path, kind) + FullLoader.add_path_resolver(tag, path, kind) + UnsafeLoader.add_path_resolver(tag, path, kind) else: Loader.add_path_resolver(tag, path, kind) Dumper.add_path_resolver(tag, path, kind) @@ -306,9 +321,8 @@ def add_constructor(tag, constructor, Loader=None): and a node object and produces the corresponding Python object. """ if Loader is None: - loader.Loader.add_constructor(tag, constructor) - loader.FullLoader.add_constructor(tag, constructor) - loader.UnsafeLoader.add_constructor(tag, constructor) + FullLoader.add_constructor(tag, constructor) + UnsafeLoader.add_constructor(tag, constructor) else: Loader.add_constructor(tag, constructor) @@ -320,9 +334,8 @@ def add_multi_constructor(tag_prefix, multi_constructor, Loader=None): and a node object and produces the corresponding Python object. """ if Loader is None: - loader.Loader.add_multi_constructor(tag_prefix, multi_constructor) - loader.FullLoader.add_multi_constructor(tag_prefix, multi_constructor) - loader.UnsafeLoader.add_multi_constructor(tag_prefix, multi_constructor) + FullLoader.add_multi_constructor(tag_prefix, multi_constructor) + UnsafeLoader.add_multi_constructor(tag_prefix, multi_constructor) else: Loader.add_multi_constructor(tag_prefix, multi_constructor) @@ -367,7 +380,7 @@ class YAMLObject(metaclass=YAMLObjectMetaclass): __slots__ = () # no direct instantiation, so allow immutable subclasses - yaml_loader = [Loader, FullLoader, UnsafeLoader] + yaml_loader = [FullLoader, UnsafeLoader] yaml_dumper = Dumper yaml_tag = None diff --git a/lib/yaml/cyaml.py b/lib/yaml/cyaml.py index 0c2134587..c408a1770 100644 --- a/lib/yaml/cyaml.py +++ b/lib/yaml/cyaml.py @@ -1,6 +1,6 @@ __all__ = [ - 'CBaseLoader', 'CSafeLoader', 'CFullLoader', 'CUnsafeLoader', 'CLoader', + 'CBaseLoader', 'CSafeLoader', 'CFullLoader', 'CUnsafeLoader', 'CBaseDumper', 'CSafeDumper', 'CDumper' ] @@ -12,6 +12,7 @@ from .representer import * from .resolver import * +import warnings class CBaseLoader(CParser, BaseConstructor, BaseResolver): @@ -37,17 +38,15 @@ def __init__(self, stream): class CUnsafeLoader(CParser, UnsafeConstructor, Resolver): def __init__(self, stream): + warnings.warn( + "CUnsafeLoader is unsafe and can execute arbitrary code when loading untrusted YAML data. " + "Use CSafeLoader or CFullLoader instead.", + RuntimeWarning, stacklevel=2 + ) CParser.__init__(self, stream) UnsafeConstructor.__init__(self) Resolver.__init__(self) -class CLoader(CParser, Constructor, Resolver): - - def __init__(self, stream): - CParser.__init__(self, stream) - Constructor.__init__(self) - Resolver.__init__(self) - class CBaseDumper(CEmitter, BaseRepresenter, BaseResolver): def __init__(self, stream, diff --git a/lib/yaml/loader.py b/lib/yaml/loader.py index e90c11224..c0700a708 100644 --- a/lib/yaml/loader.py +++ b/lib/yaml/loader.py @@ -1,5 +1,5 @@ -__all__ = ['BaseLoader', 'FullLoader', 'SafeLoader', 'Loader', 'UnsafeLoader'] +__all__ = ['BaseLoader', 'FullLoader', 'SafeLoader', 'UnsafeLoader'] from .reader import * from .scanner import * @@ -7,6 +7,7 @@ from .composer import * from .constructor import * from .resolver import * +import warnings class BaseLoader(Reader, Scanner, Parser, Composer, BaseConstructor, BaseResolver): @@ -38,23 +39,16 @@ def __init__(self, stream): SafeConstructor.__init__(self) Resolver.__init__(self) -class Loader(Reader, Scanner, Parser, Composer, Constructor, Resolver): - - def __init__(self, stream): - Reader.__init__(self, stream) - Scanner.__init__(self) - Parser.__init__(self) - Composer.__init__(self) - Constructor.__init__(self) - Resolver.__init__(self) - -# UnsafeLoader is the same as Loader (which is and was always unsafe on -# untrusted input). Use of either Loader or UnsafeLoader should be rare, since -# FullLoad should be able to load almost all YAML safely. Loader is left intact -# to ensure backwards compatibility. +# UnsafeLoader is unsafe on untrusted input. Use should be rare, since +# FullLoader should be able to load almost all YAML safely. class UnsafeLoader(Reader, Scanner, Parser, Composer, Constructor, Resolver): def __init__(self, stream): + warnings.warn( + "UnsafeLoader is unsafe and can execute arbitrary code when loading untrusted YAML data. " + "Use SafeLoader or FullLoader instead.", + RuntimeWarning, stacklevel=2 + ) Reader.__init__(self, stream) Scanner.__init__(self) Parser.__init__(self) From 409b7dc291a704c319bfd38f55a1b321c0a23fb1 Mon Sep 17 00:00:00 2001 From: banteg <4562643+banteg@users.noreply.github.com> Date: Fri, 7 Mar 2025 09:27:31 +0400 Subject: [PATCH 2/2] Update tests to use UnsafeLoader instead of Loader MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Replace yaml.Loader with yaml.UnsafeLoader in test_structure.py * Update test_yaml_ext.py to use UnsafeLoader/CUnsafeLoader instead of removed Loader class * Ensures tests still work with the removal of the Loader alias 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude --- tests/legacy_tests/test_structure.py | 6 +++--- tests/legacy_tests/test_yaml_ext.py | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/legacy_tests/test_structure.py b/tests/legacy_tests/test_structure.py index cbd4c3e03..e221cad43 100644 --- a/tests/legacy_tests/test_structure.py +++ b/tests/legacy_tests/test_structure.py @@ -38,7 +38,7 @@ def test_structure(data_filename, structure_filename, verbose=False): nodes2 = eval(file.read()) try: with open(data_filename, 'rb') as file: - loader = yaml.Loader(file) + loader = yaml.UnsafeLoader(file) while loader.check_event(): if loader.check_event( yaml.StreamStartEvent, yaml.StreamEndEvent, @@ -144,9 +144,9 @@ def test_composer(data_filename, canonical_filename, verbose=False): def _make_loader(): global MyLoader - class MyLoader(yaml.Loader): + class MyLoader(yaml.UnsafeLoader): def construct_sequence(self, node): - return tuple(yaml.Loader.construct_sequence(self, node)) + return tuple(yaml.UnsafeLoader.construct_sequence(self, node)) def construct_mapping(self, node): pairs = self.construct_pairs(node) pairs.sort(key=(lambda i: str(i))) diff --git a/tests/legacy_tests/test_yaml_ext.py b/tests/legacy_tests/test_yaml_ext.py index a2ccb2f20..e708823b4 100644 --- a/tests/legacy_tests/test_yaml_ext.py +++ b/tests/legacy_tests/test_yaml_ext.py @@ -3,7 +3,7 @@ yaml.PyBaseLoader = yaml.BaseLoader yaml.PySafeLoader = yaml.SafeLoader -yaml.PyLoader = yaml.Loader +yaml.PyLoader = yaml.UnsafeLoader yaml.PyBaseDumper = yaml.BaseDumper yaml.PySafeDumper = yaml.SafeDumper yaml.PyDumper = yaml.Dumper @@ -71,7 +71,7 @@ def new_safe_dump_all(documents, stream=None, **kwds): def _set_up(): yaml.BaseLoader = yaml.CBaseLoader yaml.SafeLoader = yaml.CSafeLoader - yaml.Loader = yaml.CLoader + yaml.UnsafeLoader = yaml.CUnsafeLoader yaml.BaseDumper = yaml.CBaseDumper yaml.SafeDumper = yaml.CSafeDumper yaml.Dumper = yaml.CDumper @@ -94,7 +94,7 @@ def _set_up(): def _tear_down(): yaml.BaseLoader = yaml.PyBaseLoader yaml.SafeLoader = yaml.PySafeLoader - yaml.Loader = yaml.PyLoader + yaml.UnsafeLoader = yaml.PyLoader yaml.BaseDumper = yaml.PyBaseDumper yaml.SafeDumper = yaml.PySafeDumper yaml.Dumper = yaml.PyDumper @@ -251,7 +251,7 @@ def test_large_file(verbose=False): for i in range(2**(SIZE_FILE-SIZE_ITERATION-SIZE_LINE) + 1): temp_file.write(bytes(('-' + (' ' * (2**SIZE_LINE-4))+ '{}\n')*(2**SIZE_ITERATION), 'utf-8')) temp_file.seek(0) - yaml.load(temp_file, Loader=yaml.CLoader) + yaml.load(temp_file, Loader=yaml.CUnsafeLoader) test_large_file.unittest = None