cookie: fix crash in netscape cookie parsing (take 2) #15826
Closed
Conversation
This file contains 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
|
I created test 3104 that crashes before this fix and works fine with it: https://github.com/curl/curl/tree/bagder/cookielist-netscape. You can either merge it with your fix, or I can do a PR after. You decide. |
jay
changed the title
- Tokenize the input string without modifying it. Prior to this change a segfault could occur if the input string was const because the tokenizer modified the input string. For example if the user set CURLOPT_COOKIELIST to a const string then libcurl would likely cause a crash when modifying that string. Even if the string was not const or a crash did not occur there was still the incorrect and unexpected modification of the user's input string. This issue was caused by 30da1f5 (precedes 8.11.0) which refactored some options parsing and eliminated the copy of the input string. Also, an earlier commit f88cc65 incorrectly cast the input pointer when passing it to strtok. Co-authored-by: Daniel Stenberg Closes #xxxx
jay
force-pushed
the
cookie_crash_take2
branch
from
ce0c76c to
c721759
Compare
Thanks. I added it and a Co-authored-by tag. |
|
Analysis of PR #15826 at c7217597: Test 2203 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Generated by Testclutch |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Prior to this change a segfault could occur if the input string was const because the tokenizer modified the input string. For example if the user set CURLOPT_COOKIELIST to a const string then libcurl would likely cause a crash when modifying that string. Even if the string was not const or a crash did not occur there was still the incorrect and unexpected modification of the user's input string.
This issue was caused by 30da1f5 (precedes 8.11.0) which refactored some options parsing and eliminated the copy of the input string. Also, an earlier commit f88cc65 incorrectly cast the input pointer when passing it to strtok.
Closes #xxxx
Alternate take of #15824