gh-148178: Validate async debug offsets read from remote process#148179
gh-148178: Validate async debug offsets read from remote process#148179gpshead wants to merge 2 commits intopython:mainfrom
Conversation
|
I need to think a bit more about how to do this properly. We need to validate every field of every section, not just the async debug. I also need to think carefully how to do this without having to write all by hand and how to ensure we fail at build time if we forget. |
|
@gpshead I was working on a fix while you opened this. If you want we can iterate over this draft but since there is a lot more to do here and is quite tricky because I suspect we are going to need a lot of macros I am not sure if that's going to be the best route. |
The _remote_debugging module reads async_debug_offsets from the target process's memory but did not validate them, unlike debug_offsets which go through validate_debug_offsets(). The asyncio_task_object.size field is used as the read length into fixed-size 4096-byte stack buffers (SIZEOF_TASK_OBJ); a malicious or compromised target process could supply a larger size and overflow the debugger's stack. Add validate_async_debug_offsets() and call it from read_async_debug() (the single chokepoint for loading these offsets) to bound the task object size and the member offsets that index into the local buffer.
gpshead
force-pushed
the
fix-async-debug-offsets-validation
branch
from
2943185 to
83bda8a
Compare
|
Totally fine if this PR gets closed. I mostly i wanted to surface it as food for thought (I suppose I could've just linked to the branch for that without a draft PR). |
Thanks a lot! I really appreciate the help and I want to say this explicitly :) It's just that unfortunately this fix will be tricky to balance in all fronts :( |
|
Closing in favour of #148187 Thanks for the draft Greg! |
2 participants
The _remote_debugging module reads async_debug_offsets from the target process's memory but did not validate them, unlike debug_offsets which go through validate_debug_offsets(). The asyncio_task_object.size field is used as the read length into fixed-size 4096-byte stack buffers (SIZEOF_TASK_OBJ); a malicious or compromised target process could supply a larger size and overflow the debugger's stack.
Add validate_async_debug_offsets() and call it from read_async_debug() (the single chokepoint for loading these offsets) to bound the task object size and the member offsets that index into the local buffer.
--
Opening as a draft to start with as this isn't my area of the code and there are possibly more of these to be fixed. Claude authored the fix.