Skip to content
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

Make YAML storage loading safer and able to catch errors #206

Merged

Conversation

arch1t3cht
Copy link
Member

@arch1t3cht arch1t3cht commented Oct 4, 2024

See the individual commit messages for more information.

This prevents arbitrary code execution from access to config files
using tags like !!python/object/apply:os.system and in the process
sets up some infrastructure for eventually switching to storage using
custom tags that does not use python/object.
It does, however, need a bit of yaml constructor spaghetti to parse
some of the datatypes that can't simply inherit from SafeYAMLObject.
This prevents storage loading from erroring out when loading an old
storage that references classes that no longer exist. (vspreview may
still error out later when it gets None in places where it doesn't
expect it, but that can then be handled with additional checks)

This fixes one of the two causes of crashes due to outdated storage
files (the other one being aliases in the local storage pointing to
removed fields in the global storage).
This fixes the second set of crashes due to outdated config files.
One unfortunate consequence is that we can no longer use the C loaders,
since we need to override methods. If this has a noticable performance
impact, one could first try loading using the C loader and then fall
back to the loader wrapper in the case of errors.

Also, some of this code no longer feels like it belongs in bases.py but
oh well.
@arch1t3cht arch1t3cht changed the title Switch to safe YAML loader Make YAML storage loading safer and able to catch errors Oct 4, 2024
@LightArrowsEXE LightArrowsEXE merged commit 7c24aa8 into Jaded-Encoding-Thaumaturgy:master Oct 4, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants