-
-
Notifications
You must be signed in to change notification settings - Fork 842
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
[Bug]: Possible race condition in WinHTTP backend #12563
Labels
bug
Something isn't working
Comments
JGRennison
added a commit
to JGRennison/Upstream-OpenTTD
that referenced
this issue
May 11, 2024
rubidium42
pushed a commit
that referenced
this issue
May 23, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Version of OpenTTD
master
Expected result
No possible race conditions, etc.
Actual result
In the WinHTTP backend there are various instances of this pattern:
where
this
is a NetworkHTTPRequest.This appears to be a racy (albeit with a narrow window), because finished is made true before is the callback queue is appended to. If the callback queue was previously empty, it would appear that the NetworkHTTPRequest instance is liable for deletion via
NetworkHTTPSocketHandler::HTTPReceive
andNetworkHTTPRequest::Receive
in the gap after finished is set to true, but before callback.OnFailure is called. It would seem that this could result in the call tocallback.OnFailure
being a use after free because*this
has already been destructed.Probably
finished
should be assigned after callingcallback.OnFailure
(i.e. release semantics), and inNetworkHTTPRequest::Receive
it should be read once, first (i.e. acquire semantics).Steps to reproduce
See http_winhttp.cpp
The text was updated successfully, but these errors were encountered: