Conversation
|
Analysis of PR #22500 at 3326a069: Test 3103 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 492 different CI jobs (the link just goes to one of them). Test 3104 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 433 different CI jobs (the link just goes to one of them). Test 506 failed, which has NOT been flaky recently, so there could be a real issue in this PR. Note that this test has failed in 40 different CI jobs (the link just goes to one of them). Generated by Testclutch |
There was a problem hiding this comment.
Pull request overview
This PR strengthens curl’s cookie handling by rejecting cookies loaded from a cookie file when they target a Public Suffix List (PSL) domain (e.g., .co.uk), reducing the risk of cross-site cookie injection. It also refactors internal cookie APIs to pass structured flags instead of multiple booleans, and adds a regression test to verify the PSL-domain rejection behavior.
Changes:
- Enforce PSL-domain rejection for cookies loaded from cookie files (during pre-transfer load).
- Refactor
Curl_cookie_add()andCurl_cookie_loadfiles()to use aflagsbitmask (e.g.,COOKIE_NOPSL,COOKIE_NOSESSION). - Add test 409 to validate that PSL-domain cookies from a file are refused while valid host cookies are accepted.
Reviewed changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/data/test409 | New test ensuring PSL-domain cookies from a file are rejected. |
| tests/data/Makefile.am | Registers the new test case. |
| lib/transfer.c | Loads cookie files with flags (session handling). |
| lib/setopt.c | Routes COOKIELIST operations through the new cookie flags API. |
| lib/http.c | Updates Set-Cookie processing to use the new flags API. |
| lib/cookie.h | Introduces cookie flag macros and updates function signatures. |
| lib/cookie.c | Implements PSL checks for file-loaded cookies and flag-based cookie parsing. |
| docs/libcurl/opts/CURLOPT_COOKIELIST.md | Documents PSL-bypass behavior for manual cookie insertion. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Verified by test 409 Reported-by: 1rhino2 on hackerone Closes #22500
62ac381 to
73bf594
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 8 changed files in this pull request and generated no new comments.
Suppressed comments (1)
lib/setopt.c:1567
- CURLOPT_COOKIELIST "RELOAD" now calls Curl_cookie_loadfiles() with COOKIE_NOPSL only, which means it no longer honors CURLOPT_COOKIESESSION (dropping session cookies) the way the previous implementation did (cookie_loadfiles used data->set.cookiesession internally). This is a behavior regression for applications relying on COOKIESESSION + RELOAD.
else if(curl_strequal(ptr, "RELOAD")) {
/* reload cookies from file */
return Curl_cookie_loadfiles(data, COOKIE_NOPSL);
}
Verified by test 409 Reported-by: 1rhino2 on hackerone Closes #22500
81c8fe7 to
99c8537
Compare
Verified by test 409
Reported-by: 1rhino2 on hackerone