Skip to content

cookie: fix rejection when tabs in value#21185

Closed
bagder wants to merge 1 commit intomasterfrom
bagder/fix-cookie-tab-reject
Closed

cookie: fix rejection when tabs in value#21185
bagder wants to merge 1 commit intomasterfrom
bagder/fix-cookie-tab-reject

Conversation

@bagder
Copy link
Copy Markdown
Member

@bagder bagder commented Apr 1, 2026

A previous refactor changed the TAB check so that the octet could be accepted in the 'path', which would cause an invalid line in the saved cookie file so not possible to read the cookie back. Not terrible because the path cannot contain a raw tab so it would never match anyway.

Add test 1685 to verify

Reported-by: Izan on hackerone

A previous refactor changed the TAB check so that the octet could be
accepted in the 'path', which would cause an invalid line in the saved
cookie file so not possible to read the cookie back. Not terrible
because the path cannot contain a raw tab anyway so it would never match
anyway.

Add test 1685 to verify

Reported-by: Izan on hackerone
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR fixes libcurl’s cookie parsing so cookies containing a raw TAB in a parsed name=value token (including attributes like path) are rejected early, preventing invalid tab-delimited lines from being written to the cookie jar and later becoming unreadable.

Changes:

  • Move the TAB-in-value rejection check to apply to all name=value segments in Set-Cookie parsing (including attributes like path).
  • Add regression test test1685 covering a cookie with a TAB in the path attribute and verifying only the valid cookie is persisted.
  • Register the new test in the testcases list.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
lib/cookie.c Rejects cookies containing TABs in any parsed name=value value during Set-Cookie header parsing.
tests/data/test1685 New test ensuring cookies with TAB in path are dropped and do not corrupt the saved cookie jar.
tests/data/Makefile.am Adds test1685 to the test suite’s testcase list.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bagder bagder marked this pull request as ready for review April 1, 2026 06:33
@bagder
Copy link
Copy Markdown
Member Author

bagder commented Apr 1, 2026

augment review

@augmentcode
Copy link
Copy Markdown

augmentcode bot commented Apr 1, 2026

🤖 Augment PR Summary

Summary: Fix cookie parsing so Set-Cookie pairs with HTABs in their values are rejected consistently.

Changes:

  • Move the TAB-in-val check earlier in parse_cookie_header() so it applies to all name=value pairs (including path).
  • Add regression test test1685 verifying that a cookie with a TAB in the path attribute is dropped while a subsequent valid cookie is still persisted to the cookie jar.

Technical Notes: This prevents writing malformed Netscape cookie-jar lines (tab-delimited) that cannot be read back, matching the RFC6265 cookie-octet restrictions.

🤖 Was this summary useful? React with 👍 or 👎

Copy link
Copy Markdown

@augmentcode augmentcode bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Review completed. No suggestions at this time.

Comment augment review to trigger a new review at any time.

@bagder bagder closed this in 8e8bdd3 Apr 1, 2026
@bagder bagder deleted the bagder/fix-cookie-tab-reject branch April 1, 2026 08:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Development

Successfully merging this pull request may close these issues.

2 participants