-
Notifications
You must be signed in to change notification settings - Fork 519
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevents arbitrary code execution during python/object/new constructor #386
Conversation
What shall I do to fix travis? |
CVE-2020-1747 has been assigned to this flaw. |
Thanks! |
@ingydotnet my suggestion would be to prepare 5.3.1 with this fix. |
It looks like lib/yaml/contructor.py needs the same/similar changes too. |
Ah, yes right. I forgot about python2. |
Changed |
I'm not part of the upstream team, so don't take this as any kind of direction to get this merged, but tests would be nice too. It would make it less likely that this issue would recur in the future. Scott K |
Thanks for the suggestion! Added a test as well. Is there anything else I can do to improve the PR? |
I requested a review from @ingydotnet , I messaged him in two IRC channels and in a private message, and sent a message on his phone. |
@ingydotnet currently has limited internet access and will look at it as soon as possible. |
d9bb589
to
a2eee01
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the work. I want to get this right because it is user facing and harder to change later.
43c1a0e
to
fdc125f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for your timely worked. I wish we were in more amenable timezones. :)
Address the "private" method concern, and I think we'll be good. Cheers.
In FullLoader python/object/new constructor, implemented by construct_python_object_apply, has support for setting the state of a deserialized instance through the set_python_instance_state method. After setting the state, some operations are performed on the instance to complete its initialization, however it is possible for an attacker to set the instance' state in such a way that arbitrary code is executed by the FullLoader. This patch tries to block such attacks in FullLoader by preventing set_python_instance_state from setting arbitrary properties. It implements a blacklist that includes `extend` method (called by construct_python_object_apply) and all special methods (e.g. __set__, __setitem__, etc.). Users who need special attributes being set in the state of a deserialized object can still do it through the UnsafeLoader, which however should not be used on untrusted input. Additionally, they can subclass FullLoader and redefine `get_state_keys_blacklist()` to extend/replace the list of blacklisted keys, passing the subclassed loader to yaml.load.
fdc125f
to
1295491
Compare
Sorry. Looks like you did fix that. Not sure how I missed it. I'll approve this. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
def get_state_keys_blacklist(self): | ||
return ['^extend$', '^__.*__$'] | ||
|
||
def get_state_keys_blacklist_regexp(self): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure this needs to be public method. But I'll go ahead and approve it. :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Try my suggested fix to get travis et al passing.
1295491
to
9091565
Compare
Great, thanks! |
Thanks @ret2libc |
#386) * Prevents arbitrary code execution during python/object/new constructor In FullLoader python/object/new constructor, implemented by construct_python_object_apply, has support for setting the state of a deserialized instance through the set_python_instance_state method. After setting the state, some operations are performed on the instance to complete its initialization, however it is possible for an attacker to set the instance' state in such a way that arbitrary code is executed by the FullLoader. This patch tries to block such attacks in FullLoader by preventing set_python_instance_state from setting arbitrary properties. It implements a blacklist that includes `extend` method (called by construct_python_object_apply) and all special methods (e.g. __set__, __setitem__, etc.). Users who need special attributes being set in the state of a deserialized object can still do it through the UnsafeLoader, which however should not be used on untrusted input. Additionally, they can subclass FullLoader and redefine `get_state_keys_blacklist()` to extend/replace the list of blacklisted keys, passing the subclassed loader to yaml.load. * Make sure python/object/new constructor does not set some properties * Add test to show how to subclass FullLoader with new blacklist
Fixes the following security issue: 386: Prevents arbitrary code execution during python/object/new constructor yaml/pyyaml#386 The hash of the license file changed due to the following diff: -Copyright (c) 2017-2019 Ingy döt Net +Copyright (c) 2017-2020 Ingy döt Net Signed-off-by: James Hilliard <james.hilliard1@gmail.com> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@bootlin.com> (cherry picked from commit 9063df4) [Peter: mention security impact] Signed-off-by: Peter Korsgaard <peter@korsgaard.com>
Hello, The tool still finds CVE-2020-1747 issue in _yaml.cp36-win_amd64.pyd . May i know if that's fixed in the pyd file as well? |
This release contains a security fix for CVE-2020-1747. FullLoader was still exploitable for arbitrary command execution. https://bugzilla.redhat.com/show_bug.cgi?id=1807367 Thanks to Riccardo Schirone (https://github.com/ret2libc) for both reporting this and providing the fixes to resolve it. - yaml/pyyaml#386 PR: 245937 Submitted by: daniel.engberg.lists@pyret.net MFH: 2020Q2 Security: http://vuxml.freebsd.org/freebsd/aae8fecf-888e-11ea-9714-08002718de91.html git-svn-id: svn+ssh://svn.freebsd.org/ports/head@533167 35697150-7ecd-e111-bb59-0022644237b5
This release contains a security fix for CVE-2020-1747. FullLoader was still exploitable for arbitrary command execution. https://bugzilla.redhat.com/show_bug.cgi?id=1807367 Thanks to Riccardo Schirone (https://github.com/ret2libc) for both reporting this and providing the fixes to resolve it. - yaml/pyyaml#386 PR: 245937 Submitted by: daniel.engberg.lists@pyret.net MFH: 2020Q2 Security: http://vuxml.freebsd.org/freebsd/aae8fecf-888e-11ea-9714-08002718de91.html
This release contains a security fix for CVE-2020-1747. FullLoader was still exploitable for arbitrary command execution. https://bugzilla.redhat.com/show_bug.cgi?id=1807367 Thanks to Riccardo Schirone (https://github.com/ret2libc) for both reporting this and providing the fixes to resolve it. - yaml/pyyaml#386 PR: 245937 Submitted by: daniel.engberg.lists@pyret.net MFH: 2020Q2 Security: http://vuxml.freebsd.org/freebsd/aae8fecf-888e-11ea-9714-08002718de91.html git-svn-id: svn+ssh://svn.freebsd.org/ports/head@533167 35697150-7ecd-e111-bb59-0022644237b5
Update to 5.3.1 This release contains a security fix for CVE-2020-1747. FullLoader was still exploitable for arbitrary command execution. https://bugzilla.redhat.com/show_bug.cgi?id=1807367 Thanks to Riccardo Schirone (https://github.com/ret2libc) for both reporting this and providing the fixes to resolve it. - yaml/pyyaml#386 PR: 245937 Submitted by: daniel.engberg.lists@pyret.net Security: http://vuxml.freebsd.org/freebsd/aae8fecf-888e-11ea-9714-08002718de91.html Approved by: portmgr (joneum)
https://github.com/yaml/pyyaml/blob/d0d660d035905d9c49fc0f8dafb579d2cc68c0c8/CHANGES#L7 5.3.1 (2020-03-18) * yaml/pyyaml#386 -- Prevents arbitrary code execution during python/object/new constructor 5.3 (2020-01-06) * yaml/pyyaml#290 -- Use `is` instead of equality for comparing with `None` * yaml/pyyaml#270 -- fix typos and stylistic nit * yaml/pyyaml#309 -- Fix up small typo * yaml/pyyaml#161 -- Fix handling of __slots__ * yaml/pyyaml#358 -- Allow calling add_multi_constructor with None * yaml/pyyaml#285 -- Add use of safe_load() function in README * yaml/pyyaml#351 -- Fix reader for Unicode code points over 0xFFFF * yaml/pyyaml#360 -- Enable certain unicode tests when maxunicode not > 0xffff * yaml/pyyaml#359 -- Use full_load in yaml-highlight example * yaml/pyyaml#244 -- Document that PyYAML is implemented with Cython * yaml/pyyaml#329 -- Fix for Python 3.10 * yaml/pyyaml#310 -- increase size of index, line, and column fields * yaml/pyyaml#260 -- remove some unused imports * yaml/pyyaml#163 -- Create timezone-aware datetimes when parsed as such * yaml/pyyaml#363 -- Add tests for timezone 5.2 (2019-12-02) ------------------ * Repair incompatibilities introduced with 5.1. The default Loader was changed, but several methods like add_constructor still used the old default yaml/pyyaml#279 -- A more flexible fix for custom tag constructors yaml/pyyaml#287 -- Change default loader for yaml.add_constructor yaml/pyyaml#305 -- Change default loader for add_implicit_resolver, add_path_resolver * Make FullLoader safer by removing python/object/apply from the default FullLoader yaml/pyyaml#347 -- Move constructor for object/apply to UnsafeConstructor * Fix bug introduced in 5.1 where quoting went wrong on systems with sys.maxunicode <= 0xffff yaml/pyyaml#276 -- Fix logic for quoting special characters * Other PRs: yaml/pyyaml#280 -- Update CHANGES for 5.1
https://github.com/yaml/pyyaml/blob/d0d660d035905d9c49fc0f8dafb579d2cc68c0c8/CHANGES#L7 5.3.1 (2020-03-18) * yaml/pyyaml#386 -- Prevents arbitrary code execution during python/object/new constructor 5.3 (2020-01-06) * yaml/pyyaml#290 -- Use `is` instead of equality for comparing with `None` * yaml/pyyaml#270 -- fix typos and stylistic nit * yaml/pyyaml#309 -- Fix up small typo * yaml/pyyaml#161 -- Fix handling of __slots__ * yaml/pyyaml#358 -- Allow calling add_multi_constructor with None * yaml/pyyaml#285 -- Add use of safe_load() function in README * yaml/pyyaml#351 -- Fix reader for Unicode code points over 0xFFFF * yaml/pyyaml#360 -- Enable certain unicode tests when maxunicode not > 0xffff * yaml/pyyaml#359 -- Use full_load in yaml-highlight example * yaml/pyyaml#244 -- Document that PyYAML is implemented with Cython * yaml/pyyaml#329 -- Fix for Python 3.10 * yaml/pyyaml#310 -- increase size of index, line, and column fields * yaml/pyyaml#260 -- remove some unused imports * yaml/pyyaml#163 -- Create timezone-aware datetimes when parsed as such * yaml/pyyaml#363 -- Add tests for timezone 5.2 (2019-12-02) ------------------ * Repair incompatibilities introduced with 5.1. The default Loader was changed, but several methods like add_constructor still used the old default yaml/pyyaml#279 -- A more flexible fix for custom tag constructors yaml/pyyaml#287 -- Change default loader for yaml.add_constructor yaml/pyyaml#305 -- Change default loader for add_implicit_resolver, add_path_resolver * Make FullLoader safer by removing python/object/apply from the default FullLoader yaml/pyyaml#347 -- Move constructor for object/apply to UnsafeConstructor * Fix bug introduced in 5.1 where quoting went wrong on systems with sys.maxunicode <= 0xffff yaml/pyyaml#276 -- Fix logic for quoting special characters * Other PRs: yaml/pyyaml#280 -- Update CHANGES for 5.1
https://github.com/yaml/pyyaml/blob/d0d660d035905d9c49fc0f8dafb579d2cc68c0c8/CHANGES#L7 5.3.1 (2020-03-18) * yaml/pyyaml#386 -- Prevents arbitrary code execution during python/object/new constructor 5.3 (2020-01-06) * yaml/pyyaml#290 -- Use `is` instead of equality for comparing with `None` * yaml/pyyaml#270 -- fix typos and stylistic nit * yaml/pyyaml#309 -- Fix up small typo * yaml/pyyaml#161 -- Fix handling of __slots__ * yaml/pyyaml#358 -- Allow calling add_multi_constructor with None * yaml/pyyaml#285 -- Add use of safe_load() function in README * yaml/pyyaml#351 -- Fix reader for Unicode code points over 0xFFFF * yaml/pyyaml#360 -- Enable certain unicode tests when maxunicode not > 0xffff * yaml/pyyaml#359 -- Use full_load in yaml-highlight example * yaml/pyyaml#244 -- Document that PyYAML is implemented with Cython * yaml/pyyaml#329 -- Fix for Python 3.10 * yaml/pyyaml#310 -- increase size of index, line, and column fields * yaml/pyyaml#260 -- remove some unused imports * yaml/pyyaml#163 -- Create timezone-aware datetimes when parsed as such * yaml/pyyaml#363 -- Add tests for timezone 5.2 (2019-12-02) ------------------ * Repair incompatibilities introduced with 5.1. The default Loader was changed, but several methods like add_constructor still used the old default yaml/pyyaml#279 -- A more flexible fix for custom tag constructors yaml/pyyaml#287 -- Change default loader for yaml.add_constructor yaml/pyyaml#305 -- Change default loader for add_implicit_resolver, add_path_resolver * Make FullLoader safer by removing python/object/apply from the default FullLoader yaml/pyyaml#347 -- Move constructor for object/apply to UnsafeConstructor * Fix bug introduced in 5.1 where quoting went wrong on systems with sys.maxunicode <= 0xffff yaml/pyyaml#276 -- Fix logic for quoting special characters * Other PRs: yaml/pyyaml#280 -- Update CHANGES for 5.1
Update to 5.3.1 This release contains a security fix for CVE-2020-1747. FullLoader was still exploitable for arbitrary command execution. https://bugzilla.redhat.com/show_bug.cgi?id=1807367 Thanks to Riccardo Schirone (https://github.com/ret2libc) for both reporting this and providing the fixes to resolve it. - yaml/pyyaml#386 PR: 245937 Submitted by: daniel.engberg.lists@pyret.net Security: http://vuxml.freebsd.org/freebsd/aae8fecf-888e-11ea-9714-08002718de91.html Approved by: portmgr (joneum) (cherry picked from commit ed0efb6)
This release contains a security fix for CVE-2020-1747. FullLoader was still exploitable for arbitrary command execution. https://bugzilla.redhat.com/show_bug.cgi?id=1807367 Thanks to Riccardo Schirone (https://github.com/ret2libc) for both reporting this and providing the fixes to resolve it. - yaml/pyyaml#386 PR: 245937 Submitted by: daniel.engberg.lists@pyret.net MFH: 2020Q2 Security: http://vuxml.freebsd.org/freebsd/aae8fecf-888e-11ea-9714-08002718de91.html
Update to 5.3.1 This release contains a security fix for CVE-2020-1747. FullLoader was still exploitable for arbitrary command execution. https://bugzilla.redhat.com/show_bug.cgi?id=1807367 Thanks to Riccardo Schirone (https://github.com/ret2libc) for both reporting this and providing the fixes to resolve it. - yaml/pyyaml#386 PR: 245937 Submitted by: daniel.engberg.lists@pyret.net Security: http://vuxml.freebsd.org/freebsd/aae8fecf-888e-11ea-9714-08002718de91.html Approved by: portmgr (joneum)
- Update from 3.13 to 6.0 - Update of rootfile - Changelog 6.0 (2021-10-13) * yaml/pyyaml#327 -- Change README format to Markdown * yaml/pyyaml#483 -- Add a test for YAML 1.1 types * yaml/pyyaml#497 -- fix float resolver to ignore `.` and `._` * yaml/pyyaml#550 -- drop Python 2.7 * yaml/pyyaml#553 -- Fix spelling of “hexadecimal” * yaml/pyyaml#556 -- fix representation of Enum subclasses * yaml/pyyaml#557 -- fix libyaml extension compiler warnings * yaml/pyyaml#560 -- fix ResourceWarning on leaked file descriptors * yaml/pyyaml#561 -- always require `Loader` arg to `yaml.load()` * yaml/pyyaml#564 -- remove remaining direct distutils usage 5.4.1 (2021-01-20) * yaml/pyyaml#480 -- Fix stub compat with older pyyaml versions that may unwittingly load it 5.4 (2021-01-19) * yaml/pyyaml#407 -- Build modernization, remove distutils, fix metadata, build wheels, CI to GHA * yaml/pyyaml#472 -- Fix for CVE-2020-14343, moves arbitrary python tags to UnsafeLoader * yaml/pyyaml#441 -- Fix memory leak in implicit resolver setup * yaml/pyyaml#392 -- Fix py2 copy support for timezone objects * yaml/pyyaml#378 -- Fix compatibility with Jython 5.3.1 (2020-03-18) * yaml/pyyaml#386 -- Prevents arbitrary code execution during python/object/new constructor 5.3 (2020-01-06) * yaml/pyyaml#290 -- Use `is` instead of equality for comparing with `None` * yaml/pyyaml#270 -- Fix typos and stylistic nit * yaml/pyyaml#309 -- Fix up small typo * yaml/pyyaml#161 -- Fix handling of __slots__ * yaml/pyyaml#358 -- Allow calling add_multi_constructor with None * yaml/pyyaml#285 -- Add use of safe_load() function in README * yaml/pyyaml#351 -- Fix reader for Unicode code points over 0xFFFF * yaml/pyyaml#360 -- Enable certain unicode tests when maxunicode not > 0xffff * yaml/pyyaml#359 -- Use full_load in yaml-highlight example * yaml/pyyaml#244 -- Document that PyYAML is implemented with Cython * yaml/pyyaml#329 -- Fix for Python 3.10 * yaml/pyyaml#310 -- Increase size of index, line, and column fields * yaml/pyyaml#260 -- Remove some unused imports * yaml/pyyaml#163 -- Create timezone-aware datetimes when parsed as such * yaml/pyyaml#363 -- Add tests for timezone 5.2 (2019-12-02) * Repair incompatibilities introduced with 5.1. The default Loader was changed, but several methods like add_constructor still used the old default yaml/pyyaml#279 -- A more flexible fix for custom tag constructors yaml/pyyaml#287 -- Change default loader for yaml.add_constructor yaml/pyyaml#305 -- Change default loader for add_implicit_resolver, add_path_resolver * Make FullLoader safer by removing python/object/apply from the default FullLoader yaml/pyyaml#347 -- Move constructor for object/apply to UnsafeConstructor * Fix bug introduced in 5.1 where quoting went wrong on systems with sys.maxunicode <= 0xffff yaml/pyyaml#276 -- Fix logic for quoting special characters * Other PRs: yaml/pyyaml#280 -- Update CHANGES for 5.1 5.1.2 (2019-07-30) * Re-release of 5.1 with regenerated Cython sources to build properly for Python 3.8b2+ 5.1.1 (2019-06-05) * Re-release of 5.1 with regenerated Cython sources to build properly for Python 3.8b1 5.1 (2019-03-13) * yaml/pyyaml#35 -- Some modernization of the test running * yaml/pyyaml#42 -- Install tox in a virtualenv * yaml/pyyaml#45 -- Allow colon in a plain scalar in a flow context * yaml/pyyaml#48 -- Fix typos * yaml/pyyaml#55 -- Improve RepresenterError creation * yaml/pyyaml#59 -- Resolves #57, update readme issues link * yaml/pyyaml#60 -- Document and test Python 3.6 support * yaml/pyyaml#61 -- Use Travis CI built in pip cache support * yaml/pyyaml#62 -- Remove tox workaround for Travis CI * yaml/pyyaml#63 -- Adding support to Unicode characters over codepoint 0xffff * yaml/pyyaml#75 -- add 3.12 changelog * yaml/pyyaml#76 -- Fallback to Pure Python if Compilation fails * yaml/pyyaml#84 -- Drop unsupported Python 3.3 * yaml/pyyaml#102 -- Include license file in the generated wheel package * yaml/pyyaml#105 -- Removed Python 2.6 & 3.3 support * yaml/pyyaml#111 -- Remove commented out Psyco code * yaml/pyyaml#129 -- Remove call to `ord` in lib3 emitter code * yaml/pyyaml#149 -- Test on Python 3.7-dev * yaml/pyyaml#158 -- Support escaped slash in double quotes "\/" * yaml/pyyaml#175 -- Updated link to pypi in release announcement * yaml/pyyaml#181 -- Import Hashable from collections.abc * yaml/pyyaml#194 -- Reverting yaml/pyyaml#74 * yaml/pyyaml#195 -- Build libyaml on travis * yaml/pyyaml#196 -- Force cython when building sdist * yaml/pyyaml#254 -- Allow to turn off sorting keys in Dumper (2) * yaml/pyyaml#256 -- Make default_flow_style=False * yaml/pyyaml#257 -- Deprecate yaml.load and add FullLoader and UnsafeLoader classes * yaml/pyyaml#261 -- Skip certain unicode tests when maxunicode not > 0xffff * yaml/pyyaml#263 -- Windows Appveyor build Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> --git a/config/rootfiles/packages/python3-yaml b/config/rootfiles/packages/python3-yaml x 0870a2346..bd4009a08 100644 * yaml/pyyaml#195 -- Build libyaml on travis * yaml/pyyaml#196 -- Force cython when building sdist * yaml/pyyaml#254 -- Allow to turn off sorting keys in Dumper (2) * yaml/pyyaml#256 -- Make default_flow_style=False * yaml/pyyaml#257 -- Deprecate yaml.load and add FullLoader and Uns oader classes * yaml/pyyaml#261 -- Skip certain unicode tests when maxunicode not xffff * yaml/pyyaml#263 -- Windows Appveyor build Signed-off-by: Adolf Belka <adolf.belka@ipfire.org> Reviewed-by: Peter Müller <peter.mueller@ipfire.org>
yaml#386) Original commit: 0afa8ac * Prevents arbitrary code execution during python/object/new constructor In FullLoader python/object/new constructor, implemented by construct_python_object_apply, has support for setting the state of a deserialized instance through the set_python_instance_state method. After setting the state, some operations are performed on the instance to complete its initialization, however it is possible for an attacker to set the instance' state in such a way that arbitrary code is executed by the FullLoader. This patch tries to block such attacks in FullLoader by preventing set_python_instance_state from setting arbitrary properties. It implements a blacklist that includes `extend` method (called by construct_python_object_apply) and all special methods (e.g. __set__, __setitem__, etc.). Users who need special attributes being set in the state of a deserialized object can still do it through the UnsafeLoader, which however should not be used on untrusted input. Additionally, they can subclass FullLoader and redefine `get_state_keys_blacklist()` to extend/replace the list of blacklisted keys, passing the subclassed loader to yaml.load. * Make sure python/object/new constructor does not set some properties * Add test to show how to subclass FullLoader with new blacklist
yaml#386) Original commit: 0afa8ac * Prevents arbitrary code execution during python/object/new constructor In FullLoader python/object/new constructor, implemented by construct_python_object_apply, has support for setting the state of a deserialized instance through the set_python_instance_state method. After setting the state, some operations are performed on the instance to complete its initialization, however it is possible for an attacker to set the instance' state in such a way that arbitrary code is executed by the FullLoader. This patch tries to block such attacks in FullLoader by preventing set_python_instance_state from setting arbitrary properties. It implements a blacklist that includes `extend` method (called by construct_python_object_apply) and all special methods (e.g. __set__, __setitem__, etc.). Users who need special attributes being set in the state of a deserialized object can still do it through the UnsafeLoader, which however should not be used on untrusted input. Additionally, they can subclass FullLoader and redefine `get_state_keys_blacklist()` to extend/replace the list of blacklisted keys, passing the subclassed loader to yaml.load. * Make sure python/object/new constructor does not set some properties * Add test to show how to subclass FullLoader with new blacklist
In FullLoader python/object/new constructor, implemented by
construct_python_object_apply, has support for setting the state of a
deserialized instance through the set_python_instance_state method.
After setting the state, some operations are performed on the instance
to complete its initialization, however it is possible for an attacker
to set the instance' state in such a way that arbitrary code is executed
by the FullLoader.
This patch tries to block such attacks in FullLoader by preventing
set_python_instance_state from setting arbitrary properties. It
implements a blacklist that includes
extend
method (called byconstruct_python_object_apply) and all special methods (e.g.
__set__
,__setitem__
, etc.).Users who need special attributes being set in the state of a
deserialized object can still do it through the UnsafeLoader, which
however should not be used on untrusted input. Additionally, they can
subclass FullLoader and redefine
state_blacklist_regexp
to include theadditional attributes they need, passing the subclassed loader to
yaml.load.