Use a safe YAML loader for pretrained configs while preserving tuple support#59
Open
praneethhere wants to merge 1 commit into
Open
Use a safe YAML loader for pretrained configs while preserving tuple support#59praneethhere wants to merge 1 commit into
praneethhere wants to merge 1 commit into
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
This PR hardens pretrained-config loading in
TribeModel.from_pretrained().Today the code reads
config.yamlwithyaml.UnsafeLoader:UnsafeLoaderconstructs arbitrary Python objects from YAML tags such as!!python/object/new:os.system [...], which means a malicious ortampered
config.yaml(local file or downloaded from a Hub repo) canexecute code at load time.
This PR replaces that call with a
yaml.SafeLoadersubclass thatexplicitly opts in to only the
!!python/tupletag, which is thesingle Python-specific tag known to be used by published TRIBE configs.
All other Python-specific tags (
python/object,python/object/new,python/name,python/module, …) are rejected by the safe base class.Changes
tribev2/config_utils.pywith aTribeConfigLoader(SafeLoadersubclass) and a
load_config()helper.tribev2/demo_utils.py.tests/test_config_utils.pycovering:!!python/tuplepattern as a realtuple,!!python/object/new:os.system [...]payloads,!!python/name:*lookups.Why a custom subclass instead of
yaml.safe_loadA drop-in
yaml.safe_load(f)is what most lints suggest, but it wouldbreak loading the currently published
facebook/tribev2config becausethe existing config uses the
!!python/tupletag. TheSafeLoadersubclass is the smallest change that keeps the existing config working
while shutting the door on arbitrary code execution. The included tests
encode both halves of that contract.
Test plan