pingpong: call Curl_pp_init() in the DO actions of the users#12752
Closed
pingpong: call Curl_pp_init() in the DO actions of the users#12752
Conversation
Member
Author
|
#12757 is a bigger take that should supersede this. I keep this open until that happens, just in case. |
... and not in *connect. The pp_init function assigns the linestart_resp pointer to the current download buffer for the handle and if the transfer is performed on a reused connection, the *connect function is not called and as a result then pp_init is not called for this transfer from there. A second place that calls pp_init is the pingpong sendf() function. If pp_init is NOT called, the pingpong struct keeps the old pointer set when the connectiom was setup. As the download buffer is allocated on-demand when reaching the CONNECT state and freed in the DONE state, there is a risk that a transfer reusing a connection uses a different buffer which can lead to the the pingpong code accessing the already freed buffer. Thanks to the backup call in the send function, this bug is hard to trigger: it needs to reuse the connection, not send any commands because of an early failure and yet have the pingpong code attempt reading response data from the server. This has so far only been achieved by the fuzzer when doing FTP wildcard requests. This is a temp fix while "the real one" is being worked on => completely removing the (ab)use of the download buffer in the pingpong protocol code. Bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=66012
bagder
force-pushed
the
bagder/pp-init
branch
from
2e33925 to
bfbb964
Compare
Member
Author
|
Replaced by #12757 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
... and not in *connect.
The pp_init function stores a pointer to the current download buffer for the handle and if the transfer is performed on a reused connection, the *connect function is not called and as a result then pp_init is not called for this transfer from there.
A second place that calls pp_init is the pingpong sendf() function.
If pp_init is NOT called, the pingpong struct keeps the old pointer set when the connectiom was setup. As the download buffer is allocated on-demand when reaching the CONNECT state and freed in the DONE state, there is a risk that a transfer reusing a connection uses a different buffer which can lead to the the pingpong code accessing the already freed buffer.
Thanks to the backup call in the send function, this bug is hard to trigger: it needs to reuse the connection, not send any commands because of an early failure and yet have the pingpong code attempt reading response data from the server.
This has so far only been achieved by the fuzzer.
Bug: https://bugs.chromium.org/p/oss-fuzz/issues/detail?id=66012