cpython icon indicating copy to clipboard operation
cpython copied to clipboard

Use After Free in Stop(_lsprof.c)

Open kcatss opened this issue 1 year ago • 9 comments

What happened?

Version

Python 3.14.0a0 (remotes/origin/HEAD-dirty:bd473aa598, Jul 1 2024, 16:59:13) [GCC 9.4.0] patch from PR https://github.com/python/cpython/pull/120297 if not patched, can still trigger uaf

build with asan

 patch -p0 < patch.diff

Run the command after saving the following code to patch.diff

patch
diff --git Modules/_lsprof.c Modules/_lsprof.c
index 5cf9eba243..a049565002 100644
--- Modules/_lsprof.c
+++ Modules/_lsprof.c
@@ -311,10 +311,17 @@ initContext(ProfilerObject *pObj, ProfilerContext *self, ProfilerEntry *entry)
 }
 
 static void
-Stop(ProfilerObject *pObj, ProfilerContext *self, ProfilerEntry *entry)
+Stop(ProfilerObject *pObj, ProfilerContext **pself, ProfilerEntry *entry)
 {
+    ProfilerContext *self = *pself;
     PyTime_t tt = call_timer(pObj) - self->t0;
     PyTime_t it = tt - self->subt;
+    // call_timer could have flushed all contexts, in this case, set pself to NULL
+    // to indicate that there's nothing needs to be done for it
+    if (pObj->currentProfilerContext == NULL) {
+        *pself = NULL;
+        return;
+    }
     if (self->previous)
         self->previous->subt += tt;
     pObj->currentProfilerContext = self->previous;
@@ -395,14 +402,17 @@ ptrace_leave_call(PyObject *self, void *key)
         return;
     profEntry = getEntry(pObj, key);
     if (profEntry) {
-        Stop(pObj, pContext, profEntry);
+        Stop(pObj, &pContext, profEntry);
     }
     else {
         pObj->currentProfilerContext = pContext->previous;
     }
-    /* put pContext into the free list */
-    pContext->previous = pObj->freelistProfilerContext;
-    pObj->freelistProfilerContext = pContext;
+    /* put pContext into the free list if it's not NULL */
+    /* we need to check because Stop could potentially set pContext to NULL*/
+    if (pContext != NULL) {
+        pContext->previous = pObj->freelistProfilerContext;
+        pObj->freelistProfilerContext = pContext;
+    }
 }
 
 static int
@@ -759,11 +769,13 @@ flush_unmatched(ProfilerObject *pObj)
         ProfilerContext *pContext = pObj->currentProfilerContext;
         ProfilerEntry *profEntry= pContext->ctxEntry;
         if (profEntry)
-            Stop(pObj, pContext, profEntry);
+            Stop(pObj, &pContext, profEntry);
         else
             pObj->currentProfilerContext = pContext->previous;
-        if (pContext)
-            PyMem_Free(pContext);
+        if (pContext != NULL) {
+            pContext->previous = pObj->freelistProfilerContext;
+            pObj->freelistProfilerContext = pContext;
+        }
     }
 
 }

Root Cause

The call_timer function calls the __call__ method of evil() twice, as shown in the following call stack.

https://github.com/python/cpython/blob/c766ad206ea60b1e0edcb625b99e7631954a984f/Modules/_lsprof.c#L310

https://github.com/python/cpython/blob/c766ad206ea60b1e0edcb625b99e7631954a984f/Modules/_lsprof.c#L316

On the first call, it invokes profiler_disable(). On the second call, if the condition for self->flags is met, it returns and then calls profiler_clear(). This sequence allows all ProfilerContext to be completely freed. Consequently, after calling call_timer(pObj), self->t0 triggers a use-after-free error.

Stop(ProfilerObject *pObj, ProfilerContext **pself, ProfilerEntry *entry)
{
    ProfilerContext *self = *pself;
    PyTime_t tt = call_timer(pObj) - self->t0; // call arbitrary code, then self can be free , uaf
    PyTime_t it = tt - self->subt;  // uaf
    ...
 }

Call Stack

initContext()
    call_timer()
        profiler_disable()  <- first call
            flush_unmatched()
                Stop()
                    call_timer() 
                        profiler_disable() <- second call, return by self->flags
                        profiler_clear() <-- free all ProfilerContext
                PyTime_t tt = call_timer(pObj) - self->t0; <- after return, trigger use after free

POC

import _lsprof
class evil(dict):
    def __call__(self):
        prof.disable()
        prof.clear()
        return True
prof = _lsprof.Profiler(evil())
prof.enable()
print ("dummy")

ASAN

asan
==903512==ERROR: AddressSanitizer: heap-use-after-free on address 0x606000066e70 at pc 0x7ffff3fd19e2 bp 0x7fffffffc430 sp 0x7fffffffc420
READ of size 8 at 0x606000066e70 thread T0
    #0 0x7ffff3fd19e1 in Stop Modules/_lsprof.c:317
    #1 0x7ffff3fd1e20 in flush_unmatched Modules/_lsprof.c:772
    #2 0x7ffff3fd4052 in profiler_disable Modules/_lsprof.c:827
    #3 0x5555559a3189 in method_vectorcall_NOARGS Objects/descrobject.c:447
    #4 0x555555979751 in _PyObject_VectorcallTstate Include/internal/pycore_call.h:167
    #5 0x555555979751 in PyObject_Vectorcall Objects/call.c:327
    #6 0x555555cfb720 in _PyEval_EvalFrameDefault Python/generated_cases.c.h:879
    #7 0x555555d41505 in _PyEval_EvalFrame Include/internal/pycore_ceval.h:119
    #8 0x555555d41505 in _PyEval_Vector Python/ceval.c:1901
    #9 0x5555559788a6 in _PyFunction_Vectorcall Objects/call.c:413
    #10 0x55555597d75c in _PyObject_VectorcallDictTstate Objects/call.c:135
    #11 0x55555597dc35 in _PyObject_Call_Prepend Objects/call.c:504
    #12 0x555555b26701 in slot_tp_call Objects/typeobject.c:9700
    #13 0x555555978c70 in _PyObject_MakeTpCall Objects/call.c:242
    #14 0x7ffff3fd152e in _PyObject_VectorcallTstate Include/internal/pycore_call.h:165
    #15 0x7ffff3fd152e in _PyObject_CallNoArgs Include/internal/pycore_call.h:183
    #16 0x7ffff3fd152e in CallExternalTimer Modules/_lsprof.c:90
    #17 0x7ffff3fd2166 in call_timer Modules/_lsprof.c:121
    #18 0x7ffff3fd2166 in initContext Modules/_lsprof.c:310
    #19 0x7ffff3fd3b52 in ptrace_enter_call Modules/_lsprof.c:386
    #20 0x7ffff3fd539b in ccall_callback Modules/_lsprof.c:663
    #21 0x555555a75675 in cfunction_vectorcall_FASTCALL Objects/methodobject.c:425
    #22 0x555555e0a5df in _PyObject_VectorcallTstate Include/internal/pycore_call.h:167
    #23 0x555555e0a5df in call_one_instrument Python/instrumentation.c:907
    #24 0x555555e0beb2 in call_instrumentation_vector Python/instrumentation.c:1095
    #25 0x555555e0ff78 in _Py_call_instrumentation_2args Python/instrumentation.c:1150
    #26 0x555555d18305 in _PyEval_EvalFrameDefault Python/generated_cases.c.h:3543
    #27 0x555555d41505 in _PyEval_EvalFrame Include/internal/pycore_ceval.h:119
    #28 0x555555d41505 in _PyEval_Vector Python/ceval.c:1901
    #29 0x555555d41726 in PyEval_EvalCode Python/ceval.c:624
    #30 0x555555e6016f in run_eval_code_obj Python/pythonrun.c:1292
    #31 0x555555e630da in run_mod Python/pythonrun.c:1377
    #32 0x555555e63ec0 in pyrun_file Python/pythonrun.c:1210
    #33 0x555555e6639b in _PyRun_SimpleFileObject Python/pythonrun.c:459
    #34 0x555555e6688f in _PyRun_AnyFileObject Python/pythonrun.c:77
    #35 0x555555ecc247 in pymain_run_file_obj Modules/main.c:357
    #36 0x555555eceb5d in pymain_run_file Modules/main.c:376
    #37 0x555555ecf76e in pymain_run_python Modules/main.c:640
    #38 0x555555ecf8fe in Py_RunMain Modules/main.c:719
    #39 0x555555ecfae5 in pymain_main Modules/main.c:749
    #40 0x555555ecfe4c in Py_BytesMain Modules/main.c:773
    #41 0x5555557d6a85 in main Programs/python.c:15
    #42 0x7ffff7230082 in __libc_start_main ../csu/libc-start.c:308
    #43 0x5555557d69bd in _start (/home/kk/projects/cpython/python+0x2829bd)

0x606000066e70 is located 16 bytes inside of 56-byte region [0x606000066e60,0x606000066e98)
freed by thread T0 here:
    #0 0x7ffff768840f in __interceptor_free ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:122
    #1 0x555555a8b945 in _PyMem_RawFree Objects/obmalloc.c:90
    #2 0x555555a8dbff in _PyMem_DebugRawFree Objects/obmalloc.c:2754
    #3 0x555555a8e440 in _PyMem_DebugFree Objects/obmalloc.c:2891
    #4 0x555555ac7d45 in PyMem_Free Objects/obmalloc.c:1010
    #5 0x7ffff3fd0d21 in clearEntries Modules/_lsprof.c:282
    #6 0x7ffff3fd0dba in profiler_clear Modules/_lsprof.c:845
    #7 0x5555559a3189 in method_vectorcall_NOARGS Objects/descrobject.c:447
    #8 0x555555979751 in _PyObject_VectorcallTstate Include/internal/pycore_call.h:167
    #9 0x555555979751 in PyObject_Vectorcall Objects/call.c:327
    #10 0x555555cfb720 in _PyEval_EvalFrameDefault Python/generated_cases.c.h:879
    #11 0x555555d41505 in _PyEval_EvalFrame Include/internal/pycore_ceval.h:119
    #12 0x555555d41505 in _PyEval_Vector Python/ceval.c:1901
    #13 0x5555559788a6 in _PyFunction_Vectorcall Objects/call.c:413
    #14 0x55555597d75c in _PyObject_VectorcallDictTstate Objects/call.c:135
    #15 0x55555597dc35 in _PyObject_Call_Prepend Objects/call.c:504
    #16 0x555555b26701 in slot_tp_call Objects/typeobject.c:9700
    #17 0x555555978c70 in _PyObject_MakeTpCall Objects/call.c:242
    #18 0x7ffff3fd152e in _PyObject_VectorcallTstate Include/internal/pycore_call.h:165
    #19 0x7ffff3fd152e in _PyObject_CallNoArgs Include/internal/pycore_call.h:183
    #20 0x7ffff3fd152e in CallExternalTimer Modules/_lsprof.c:90
    #21 0x7ffff3fd1702 in call_timer Modules/_lsprof.c:121
    #22 0x7ffff3fd1702 in Stop Modules/_lsprof.c:317
    #23 0x7ffff3fd1e20 in flush_unmatched Modules/_lsprof.c:772
    #24 0x7ffff3fd4052 in profiler_disable Modules/_lsprof.c:827
    #25 0x5555559a3189 in method_vectorcall_NOARGS Objects/descrobject.c:447
    #26 0x555555979751 in _PyObject_VectorcallTstate Include/internal/pycore_call.h:167
    #27 0x555555979751 in PyObject_Vectorcall Objects/call.c:327
    #28 0x555555cfb720 in _PyEval_EvalFrameDefault Python/generated_cases.c.h:879
    #29 0x555555d41505 in _PyEval_EvalFrame Include/internal/pycore_ceval.h:119
    #30 0x555555d41505 in _PyEval_Vector Python/ceval.c:1901
    #31 0x5555559788a6 in _PyFunction_Vectorcall Objects/call.c:413
    #32 0x55555597d75c in _PyObject_VectorcallDictTstate Objects/call.c:135
    #33 0x55555597dc35 in _PyObject_Call_Prepend Objects/call.c:504
    #34 0x555555b26701 in slot_tp_call Objects/typeobject.c:9700
    #35 0x555555978c70 in _PyObject_MakeTpCall Objects/call.c:242
    #36 0x7ffff3fd152e in _PyObject_VectorcallTstate Include/internal/pycore_call.h:165
    #37 0x7ffff3fd152e in _PyObject_CallNoArgs Include/internal/pycore_call.h:183
    #38 0x7ffff3fd152e in CallExternalTimer Modules/_lsprof.c:90

previously allocated by thread T0 here:
    #0 0x7ffff7688808 in __interceptor_malloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cc:144
    #1 0x555555a8c4f9 in _PyMem_RawMalloc Objects/obmalloc.c:62
    #2 0x555555a8b821 in _PyMem_DebugRawAlloc Objects/obmalloc.c:2686
    #3 0x555555a8b889 in _PyMem_DebugRawMalloc Objects/obmalloc.c:2719
    #4 0x555555a8e482 in _PyMem_DebugMalloc Objects/obmalloc.c:2876
    #5 0x555555ac7c01 in PyMem_Malloc Objects/obmalloc.c:981
    #6 0x7ffff3fd3b95 in ptrace_enter_call Modules/_lsprof.c:380
    #7 0x7ffff3fd539b in ccall_callback Modules/_lsprof.c:663
    #8 0x555555a75675 in cfunction_vectorcall_FASTCALL Objects/methodobject.c:425
    #9 0x555555e0a5df in _PyObject_VectorcallTstate Include/internal/pycore_call.h:167
    #10 0x555555e0a5df in call_one_instrument Python/instrumentation.c:907
    #11 0x555555e0beb2 in call_instrumentation_vector Python/instrumentation.c:1095
    #12 0x555555e0ff78 in _Py_call_instrumentation_2args Python/instrumentation.c:1150
    #13 0x555555d18305 in _PyEval_EvalFrameDefault Python/generated_cases.c.h:3543
    #14 0x555555d41505 in _PyEval_EvalFrame Include/internal/pycore_ceval.h:119
    #15 0x555555d41505 in _PyEval_Vector Python/ceval.c:1901
    #16 0x555555d41726 in PyEval_EvalCode Python/ceval.c:624
    #17 0x555555e6016f in run_eval_code_obj Python/pythonrun.c:1292
    #18 0x555555e630da in run_mod Python/pythonrun.c:1377
    #19 0x555555e63ec0 in pyrun_file Python/pythonrun.c:1210
    #20 0x555555e6639b in _PyRun_SimpleFileObject Python/pythonrun.c:459
    #21 0x555555e6688f in _PyRun_AnyFileObject Python/pythonrun.c:77
    #22 0x555555ecc247 in pymain_run_file_obj Modules/main.c:357
    #23 0x555555eceb5d in pymain_run_file Modules/main.c:376
    #24 0x555555ecf76e in pymain_run_python Modules/main.c:640
    #25 0x555555ecf8fe in Py_RunMain Modules/main.c:719
    #26 0x555555ecfae5 in pymain_main Modules/main.c:749
    #27 0x555555ecfe4c in Py_BytesMain Modules/main.c:773
    #28 0x5555557d6a85 in main Programs/python.c:15
    #29 0x7ffff7230082 in __libc_start_main ../csu/libc-start.c:308

SUMMARY: AddressSanitizer: heap-use-after-free Modules/_lsprof.c:317 in Stop
Shadow bytes around the buggy address:
  0x0c0c80004d70: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fd
  0x0c0c80004d80: fa fa fa fa 00 00 00 00 00 00 00 fa fa fa fa fa
  0x0c0c80004d90: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
  0x0c0c80004da0: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fa
  0x0c0c80004db0: fa fa fa fa fd fd fd fd fd fd fd fa fa fa fa fa
=>0x0c0c80004dc0: fd fd fd fd fd fd fd fa fa fa fa fa fd fd[fd]fd
  0x0c0c80004dd0: fd fd fd fa fa fa fa fa fd fd fd fd fd fd fd fa
  0x0c0c80004de0: fa fa fa fa fd fd fd fd fd fd fd fa fa fa fa fa
  0x0c0c80004df0: fd fd fd fd fd fd fd fa fa fa fa fa fd fd fd fd
  0x0c0c80004e00: fd fd fd fa fa fa fa fa fa fa fa fa fa fa fa fa
  0x0c0c80004e10: fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa fa
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07 
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
  Shadow gap:              cc
==903512==ABORTING

CPython versions tested on:

CPython main branch

Operating systems tested on:

Linux

Output from running 'python -VV' on the command line:

Python 3.14.0a0 (remotes/origin/HEAD-dirty:bd473aa598, Jul 1 2024, 16:59:13) [GCC 9.4.0]

kcatss avatar Jul 01 '24 12:07 kcatss

I have a quick PoC of my idea how we can try to solve this:

» git patch
diff --git Modules/_lsprof.c Modules/_lsprof.c
index 5cf9eba243b..54504f7a887 100644
--- Modules/_lsprof.c
+++ Modules/_lsprof.c
@@ -54,12 +54,14 @@ typedef struct {
     double externalTimerUnit;
     int tool_id;
     PyObject* missing;
+    int initializing;
 } ProfilerObject;
 
 #define POF_ENABLED     0x001
 #define POF_SUBCALLS    0x002
 #define POF_BUILTINS    0x004
 #define POF_NOMEMORY    0x100
+#define POF_IS_DISABLED 0x200
 
 /*[clinic input]
 module _lsprof
@@ -293,6 +295,7 @@ static void clearEntries(ProfilerObject *pObj)
 static void
 initContext(ProfilerObject *pObj, ProfilerContext *self, ProfilerEntry *entry)
 {
+    pObj->initializing = 1;
     self->ctxEntry = entry;
     self->subt = 0;
     self->previous = pObj->currentProfilerContext;
@@ -308,12 +311,17 @@ initContext(ProfilerObject *pObj, ProfilerContext *self, ProfilerEntry *entry)
             ++subentry->recursionLevel;
     }
     self->t0 = call_timer(pObj);
+    pObj->initializing = 0;
 }
 
 static void
 Stop(ProfilerObject *pObj, ProfilerContext *self, ProfilerEntry *entry)
 {
-    PyTime_t tt = call_timer(pObj) - self->t0;
+    if (pObj->initializing) {
+        pObj->flags |= POF_IS_DISABLED;
+    }
+    PyTime_t timer = call_timer(pObj);
+    PyTime_t tt = timer - self->t0;
     PyTime_t it = tt - self->subt;
     if (self->previous)
         self->previous->subt += tt;
@@ -408,6 +416,12 @@ ptrace_leave_call(PyObject *self, void *key)
 static int
 pending_exception(ProfilerObject *pObj)
 {
+    if (pObj->flags & POF_IS_DISABLED) {
+        pObj->flags -= POF_IS_DISABLED;
+        PyErr_SetString(PyExc_TypeError,
+                        "profiler was disabled");
+        return -1;
+    }
     if (pObj->flags & POF_NOMEMORY) {
         pObj->flags -= POF_NOMEMORY;
         PyErr_SetString(PyExc_MemoryError,
@@ -603,6 +617,9 @@ PyObject* pystart_callback(ProfilerObject* self, PyObject *const *args, Py_ssize
     PyObject* code = args[0];
     ptrace_enter_call((PyObject*)self, (void *)code, (PyObject *)code);
 
+    if (pending_exception(self)) {
+        return NULL;
+    }
     Py_RETURN_NONE;
 }
 
@@ -611,6 +628,9 @@ PyObject* pyreturn_callback(ProfilerObject* self, PyObject *const *args, Py_ssiz
     PyObject* code = args[0];
     ptrace_leave_call((PyObject*)self, (void *)code);
 
+    if (pending_exception(self)) {
+        return NULL;
+    }
     Py_RETURN_NONE;
 }
 
@@ -656,6 +676,9 @@ PyObject* ccall_callback(ProfilerObject* self, PyObject *const *args, Py_ssize_t
             Py_DECREF(cfunc);
         }
     }
+    if (pending_exception(self)) {
+        return NULL;
+    }
     Py_RETURN_NONE;
 }
 
@@ -673,6 +696,9 @@ PyObject* creturn_callback(ProfilerObject* self, PyObject *const *args, Py_ssize
             Py_DECREF(cfunc);
         }
     }
+    if (pending_exception(self)) {
+        return NULL;
+    }
     Py_RETURN_NONE;
 }

It feels a bit hacky, but:

  • All tests pass
  • Your example now fails with
» ./python.exe ex.py                
Exception ignored in: {}
Traceback (most recent call last):
  File "/Users/sobolev/Desktop/cpython2/ex.py", line 4, in __call__
    prof.disable()
TypeError: profiler was disabled
dummy

I feel like something is off.

sobolevn avatar Jul 01 '24 16:07 sobolevn

This is a dup of #120289 and the fix is in #120297

gaogaotiantian avatar Jul 01 '24 17:07 gaogaotiantian

Actually, @kcatss you are the author of #120289. Could you confirm that it's the same issue? (and the patch fixed the issue).

gaogaotiantian avatar Jul 01 '24 17:07 gaogaotiantian

@gaogaotiantian as I mentioned above, https://github.com/python/cpython/issues/120289 could not prevent this issue.

it still trigger crash after patched the PR(https://github.com/python/cpython/issues/120289)

call_timer could be called two place(InitContext, Stop, )

https://github.com/python/cpython/issues/120289 focuses on mitigating InitContext, but does not address this issue completely.

kcatss avatar Jul 01 '24 17:07 kcatss

@sobolevn

In my opinion, there are two parts to consider when mitigating dangerous arbitrary calls by user-defined callbacks:

  1. Arbitrary calls by user-defined callbacks.
  2. Reentrancy that causes side effects.

To mitigate crashes caused by these issues, set a flag before calling an arbitrary function. Then, other APIs that might be reentered by the arbitrary call can check whether the flag is set.

So, your patch looks good to me (LGTM).

kcatss avatar Jul 01 '24 17:07 kcatss

I don't think @sobolevn 's solution above solved the UAF issue. Yes an exception is reported but the UAF still happened. The exception raised will not stop call_timer() and the writeback to self->t0 still happened.

Also I don't think this is the right way to go. We should either propagate the exceptions in call_timer() or disallow users to clear the profiler manually.

gaogaotiantian avatar Jul 01 '24 19:07 gaogaotiantian

We should either propagate the exceptions in call_timer()

This seems like a good idea.

sobolevn avatar Jul 01 '24 19:07 sobolevn

If we do propagate the exception in call_timer(), we should make it simple - do now allow user to disable the profiler in the timer. We should not make the existing code too ugly just because the user can somehow do arbitrary things in the external timer.

gaogaotiantian avatar Jul 01 '24 20:07 gaogaotiantian

Okay after some thoughts I have my fix in #120297.

Here's my goal when trying to fix this:

  1. Try not make the existing code too complicated.
  2. If possible, make zero impact on performance (profiler itself is a bit performance sensitive)
  3. The solution does not have to be too elegant as this is a very dark corner - we only need to make sure UAF does not happen.

So I did not propagate the exception because it will change the existing code a lot and it's slower even when external timers are not used. I set a flag before calling the external timers and disallow the users calling disable() and clear(). The RuntimeError raised would be unraisable eventually because the timer does not propagate the exception, but because disable() and clear() does not run, no UAF happens.

The output may not be the most clean as each call to the external timer could potentially print something, but as I mentioned above, this is an evil thing and it should not happen at all. We don't need to make it pretty, just make it clear to the user that they should not do it.

gaogaotiantian avatar Jul 03 '24 01:07 gaogaotiantian

Fixed in 3.12, 3.13 and main in https://github.com/python/cpython/pull/120297

gaogaotiantian avatar Jul 18 '24 23:07 gaogaotiantian