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

asyncio: Possible missing typecheck in overlapped.c #98793

Closed
JelleZijlstra opened this issue Oct 28, 2022 · 6 comments
Closed

asyncio: Possible missing typecheck in overlapped.c #98793

JelleZijlstra opened this issue Oct 28, 2022 · 6 comments
Labels
easy OS-windows topic-asyncio type-bug An unexpected behavior, bug, or error

Comments

@JelleZijlstra
Copy link
Member

JelleZijlstra commented Oct 28, 2022

I don't have a Windows machine to check, but I noticed while reading code that the _overlapped.WSAConnect() function calls PyTuple_GET_SIZE on its argument without checking that it is actually a tuple (

switch (PyTuple_GET_SIZE(obj)) {
).

The following code should reproduce the problem and access out-of-bounds memory:

import asyncio.windows_events
import socket
ip = asyncio.windows_events.IocpProactor()
sock = socket.socket(type=socket.SOCK_DGRAM)
ip.connect(sock, None)
@JelleZijlstra JelleZijlstra added type-bug An unexpected behavior, bug, or error topic-asyncio OS-windows labels Oct 28, 2022
@serhiy-storchaka
Copy link
Member

It is checked in _overlapped.Overlapped.ConnectEx, but not in _overlapped.WSAConnect and _overlapped.Overlapped.WSASendTo.

@Fidget-Spinner
Copy link
Member

I can confirm on Windows debug build that this gives me:
Assertion failed: PyTuple_Check(op), file D:\Ken\Documents\GitHub\cpython\Include\cpython\tupleobject.h, line 23
So there needs to be a type check somewhere.

@serhiy-storchaka
Copy link
Member

I have marked it as "easy" because the fix is trivial -- just use corresponding Argument Clinic declaration in other methods as in _overlapped.Overlapped.ConnectEx. The nontrivial part is writing tests for both of affected functions. It needs an access to Windows and some time, so I left it to other contributors.

@LUCIOUS999

This comment was marked as spam.

@CharlieZhao95
Copy link
Contributor

I'm trying to create a PR for this issue and it won't take long :)

gvanrossum pushed a commit that referenced this issue Oct 30, 2022
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
CharlieZhao95 added a commit to CharlieZhao95/cpython that referenced this issue Oct 31, 2022
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
(cherry picked from commit 3ac8c0a)
CharlieZhao95 added a commit to CharlieZhao95/cpython that referenced this issue Oct 31, 2022
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
(cherry picked from commit 3ac8c0a)
gvanrossum pushed a commit that referenced this issue Oct 31, 2022
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
(cherry picked from commit 3ac8c0a)
gvanrossum pushed a commit that referenced this issue Oct 31, 2022
Co-authored-by: Kumar Aditya <59607654+kumaraditya303@users.noreply.github.com>
(cherry picked from commit 3ac8c0a)
@hauntsaninja
Copy link
Contributor

Thanks, looks like this has been completed!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
easy OS-windows topic-asyncio type-bug An unexpected behavior, bug, or error
Projects
Status: Done
Development

No branches or pull requests

6 participants