Repository navigation
Updated whitelist and Python Environment checking - #323
Open
ShadowPickle-Bench wants to merge 1 commit into
Open
ShadowPickle-Bench wants to merge 1 commit into
ShadowPickle-Bench wants to merge 1 commit into
Conversation
This branch has not been deployed
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
Integrated the whitelist from PyTorch's
weights_onlyunpickler (_weights_only_unpickler.py) into thenon_standard_importscheck (fickle.py:1295) to reduce false positive detections in pickle files. Additionally, accounted for CWE-1395 within the updated whitelist, specifically addressing the Overwritten-Modules exploit involving pickles from the ShadowPickle paper.Motivation:
Our work, ShadowPickle, demonstrates that the top 15 libraries detected during their experimental evaluation of Fickling against 3,000 benign pickle models are all present within PyTorch's
weights_onlyscanner whitelist and that Fickling does not whitelist any additional libraries beyond those covered by theweights_onlyunpickler. Furthermore, these whitelisted libraries are not known to be leveraged in the construction of malicious payloads. Incorporating them into Fickling's existing whitelist would therefore reduce the rate of false positives while preserving true positive detection performance.However, implicitly trusting third-party libraries to behave as expected introduces a notable security risk (CWE-1395) , as these libraries may be manipulated to execute malicious operations. The ShadowPickle
Overwritten-Modulesattack exemplifies this threat by overwriting entries insys.modules, redirecting the original import path of a given module (e.g.,torch.FloatStorage,collections.OrderedDict) to an attacker-controlled implementation capable of executing arbitrary payloads. Accordingly, validating the Python environment to ensure that modules are imported from their expected/usr/lib/python/locations is equally important.Changes
Added a list of libraries and modules considered part of the whitelist for the
weights_onlyunpickler:To account for module member imports, helper functions
create_full_import,is_in_weights_only, andis_sys_modules_tamperedwere introduced (fickle.py:1512) to determine whether imported modules and members belong to theweights_onlyunpickler's whitelist. Theis_sys_modules_tamperedfunction checks whether thesys.modulespath of a built-in library isn’t modified/tampered. Once confirmed that the python environment has not been tampered with, thecreate_full_importfunction constructs the full module member path (e.g.,collections.OrderedDict), which is then compared against the whitelist viais_in_weights_only.Similarly, for whole-module imports, the module path in sys.modules is checked for any case of tampering. If not, each imported name is checked against both the standard module list and the
weights_onlyunpickler whitelist:Tests
Two test cases were added to adequately validate this patch:
test_weights_only_whitelist: Verifies the scanner's behavior against all libraries included in theweights_onlyunpickler's whitelist.test_sys_modules_tampering: Verifies the scanner's ability to detect an instance ofsys.modulespath tampering.All tests in the test folder were executed and passed, confirming that default behavior remains intact.