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

Patched memo vulnerability #23

Merged
merged 1 commit into from
Dec 16, 2023
Merged

Patched memo vulnerability #23

merged 1 commit into from
Dec 16, 2023

Conversation

quasar098
Copy link
Contributor

quick fix for #22

I felt bad for solving the challenge on the Imaginary ctf but not disclosing the vulnerability, so I am making this pull request

it patches the case where the memo is not in order.

import pickle, base64

pkl = b''.join([
    pickle.UNICODE + b'os\n',
    pickle.PUT + b'2\n',
    pickle.POP,
    pickle.UNICODE + b'system\n',
    pickle.PUT + b'3\n',
    pickle.POP,
    pickle.UNICODE + b'torch\n',
    pickle.PUT + b'0\n',
    pickle.POP,
    pickle.UNICODE + b'LongStorage\n',
    pickle.PUT + b'1\n',
    pickle.POP,

    pickle.GET + b'2\n',
    pickle.GET + b'3\n',
    pickle.STACK_GLOBAL,

    pickle.MARK,
    pickle.UNICODE + b'cat flag.txt\n',
    pickle.TUPLE,

    pickle.REDUCE
]) + b'.'
print(base64.b64encode(pkl).decode())

perhaps another idea is that pickles could be checked to make sure the memo is in order (pickler always does memo from 0 counting up from what i can see, so any other order is suspicious)

@quasar098
Copy link
Contributor Author

the test appears to have failed but I do not think it was my code that did it

@mmaitre314
Copy link
Owner

First, thanks for the contribution -- I thought I would make progress on that issue but didn't. Second, the test failure looks pretty close of the code changed. It seems that memo position is accessed but does not exist (KeyError: 1 accessing memo[int(ops[n - offset][1])]). Probably a bit more fix needed. I'll try to debug (time permitting).

@mmaitre314
Copy link
Owner

Should the fix apply to PUT, BINPUT, LONG_BINPUT but not MEMOIZE? It looks like the latter has no input argument and puts the memo at the end: https://github.com/python/cpython/blob/1583c40be938d2caf363c126976bc8757df90b13/Lib/pickletools.py#L1864
On the other hand, the picketools doc mentions the other three take the memo location as input parameter and there the fix looks valid: https://github.com/python/cpython/blob/1583c40be938d2caf363c126976bc8757df90b13/Lib/pickletools.py#L1827

@quasar098
Copy link
Contributor Author

oops, i forgot about MEMOIZE i was just testing with the proof of concept vulnerability that i found on the imaginary ctf website, i will check that out

@mmaitre314
Copy link
Owner

Cool. The fix looks simpler than I expected. I can add the MEMOIZE case and some tests. Thanks for your help!

@mmaitre314 mmaitre314 merged commit ad6efbf into mmaitre314:main Dec 16, 2023
2 of 3 checks passed
@quasar098
Copy link
Contributor Author

alright

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