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

Security Fix for Arbitrary Code Execution - huntr.dev #637

Closed
wants to merge 2 commits into from

Conversation

huntr-helper
Copy link

https://huntr.dev/users/d3m0n-r00t has fixed the Arbitrary Code Execution vulnerability 🔨. Think you could fix a vulnerability like this?

Get involved at https://huntr.dev/

Q | A
Version Affected | ALL
Bug Fix | YES
Original Pull Request | 418sec#1
Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/pip/petastorm/1/README.md

User Comments:

📊 Metadata *

Fixed Arbitrary code execution in petastorm

Bounty URL: https://www.huntr.dev/bounties/1-pip-petastorm/

⚙️ Description *

Petastorm is an open source data access library developed at Uber ATG. This library enables single machine or distributed training and evaluation of deep learning models directly from datasets in Apache Parquet format. Petastorm supports popular Python-based machine learning (ML) frameworks such as Tensorflow, PyTorch, and PySpark. It can also be used from pure Python code.

Vulnerability description untrusted loading of data by the pickle.load function leading to Arbitrary code execution.

💻 Technical Description *

The function depickle_legacy_package_name_compatible() blindly loads a pickle file without any validation making it vulnerable to Arbitrary Code Execution. If the input pickle file is a malicious payload, create a file remotely.

🐛 Proof of Concept (PoC) *

import os
import pickle
#os.system('pip3 install petastorm')
from petastorm.etl import legacy
#payload formation
class ArbitraryCode:
    def __reduce__(self):
        cmd = ('xcalc')
        return os.system, (cmd,)
#Exploiting
dumps = pickle.dumps(ArbitraryCode())
legacy.depickle_legacy_package_name_compatible(dumps)

Screenshot 2021-01-05 193250

🔥 Proof of Fix (PoF) *

Screenshot 2021-01-05 193940

Fix when subprocess is called.
Screenshot 2021-01-05 195008

👍 User Acceptance Testing (UAT)

Applied fix from pickle official fix as explained in here.
https://www.cmi.ac.in/~madhavan/courses/python-2014/docs/python-3.2.1-docs-html/library/pickle.html
Proper working. (Something other than a payload).
Screenshot 2021-01-05 195158

d3m0n-r00t and others added 2 commits January 5, 2021 19:41
@CLAassistant
Copy link

CLAassistant commented Jan 21, 2021

CLA assistant check
All committers have signed the CLA.

@JamieSlome
Copy link

@d3m0n-r00t - are you able to sign the CLA? Cheers! 🍰

selitvin added a commit to selitvin/petastorm that referenced this pull request Jan 23, 2021
Based on uber#637: whitelists a set of packages which classes can be
unpickled. Prevents unpickling a malicious class that may invoke
os.execute or a similar command.
selitvin added a commit to selitvin/petastorm that referenced this pull request Jan 23, 2021
Based on uber#637: whitelists a set of packages which classes can be
unpickled. Prevents unpickling a malicious class that may invoke
os.execute or a similar command.
@selitvin
Copy link
Collaborator

Thanks for the proposed fix. I created #640 which properly fixes the issue in the context of petastorm (this PR would not pass unit tests)

@@ -43,5 +69,5 @@ def depickle_legacy_package_name_compatible(pickled_string):
'Regenerate metadata.', legacy_package_name, legacy_module, legacy_module)

pickled_string = modified_pickled_string

restricted_loads(pickled_string)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are ignoring the return value here. Also, the implementation fails the tests. Started #640 to resolve test issues.

selitvin added a commit to selitvin/petastorm that referenced this pull request Jul 25, 2021
Based on uber#637: whitelists a set of packages which classes can be
unpickled. Prevents unpickling a malicious class that may invoke
os.execute or a similar command.
selitvin added a commit to selitvin/petastorm that referenced this pull request Jul 25, 2021
Based on uber#637: whitelists a set of packages which classes can be
unpickled. Prevents unpickling a malicious class that may invoke
os.execute or a similar command.
selitvin added a commit that referenced this pull request Jul 26, 2021
Based on #637: whitelists a set of packages which classes can be
unpickled. Prevents unpickling a malicious class that may invoke
os.execute or a similar command.
selitvin added a commit to selitvin/petastorm that referenced this pull request Jul 26, 2021
Based on uber#637: whitelists a set of packages which classes can be
unpickled. Prevents unpickling a malicious class that may invoke
os.execute or a similar command.
selitvin added a commit to selitvin/petastorm that referenced this pull request Jul 26, 2021
Based on uber#637: whitelists a set of packages which classes can be
unpickled. Prevents unpickling a malicious class that may invoke
os.execute or a similar command.
selitvin added a commit that referenced this pull request Jul 26, 2021
Based on #637: whitelists a set of packages which classes can be
unpickled. Prevents unpickling a malicious class that may invoke
os.execute or a similar command.
selitvin added a commit that referenced this pull request Jul 28, 2021
Based on #637: whitelists a set of packages which classes can be
unpickled. Prevents unpickling a malicious class that may invoke
os.execute or a similar command.
@selitvin
Copy link
Collaborator

Fixed by #640. Closing this PR.

@selitvin selitvin closed this Apr 14, 2022
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.

5 participants