Repository navigation
SimpleCookie regex can make parsing extremely slow for adversarial payloads #149028
Description
Bug report
Bug description:
The current version of http.cookies has a potential performance/security regression (seemingly introduced by #113663) and detected through fuzzing, where a bad cookie payload could significantly slow down request processing on a server relying on SimpleCookie (such as bottle here or tornado there). Such a cookie could be easily be crafted by a bad actor, leading to potential denial of service.
This branch on my fork proposes an alternative regular expression that does not seem to have the problem (removed this to make it clear that my intention was not to get that merged 😅) as well as a reproducer. When run (without the regex change), it clearly shows the problem:
$ ./python reproducer_cookie_redos.py
Timing SimpleCookie().load() with increasing payload size
units bytes time
1 18 0.0001s
2 36 0.0001s
3 54 0.0003s
4 72 0.0010s
5 90 0.0042s
6 108 0.0163s
7 126 0.0648s
8 144 0.2560s
9 162 1.0294s
10 180 4.1919s
11 198 16.6055s
For reference, the reproducer is
import sys
import time
import http.cookies
_UNIT = 'x="qu\\"qu\\\\"quot<' # raw: x="qu\"qu\\"quot<
def make_payload(units: int = 11) -> str:
"""Return a Cookie header value that causes ~4^units backtrack steps."""
return (" " + _UNIT) * units
print("Timing SimpleCookie.load with increasing payload size")
print(f" {'units':>5} {'bytes':>6} {'time':>10}")
for n in range(1, 14):
payload = make_payload(n)
start = time.perf_counter()
http.cookies.SimpleCookie().load(payload)
elapsed = time.perf_counter() - start
print(f" {n:>5} {len(payload):>6} {elapsed:>10.4f}s")
if elapsed > 10:
print(" (stopping)")
breakNote I did not report this as a security issue because the original PR went out in August 2025; 3.14 was already in beta and so only the unreleased version of Python is affected. I downloaded and built 3.14 manually to make sure, and 3.14 is not affected by this regression.
The PSRT does not accept reports that only affect pre-release versions of software, as these features are considered "in-development", please open public issues. (source)
CPython versions tested on:
CPython main branch
Operating systems tested on:
Linux
Activity
tim-one commented on Apr 27, 2026
I agree this is serious, and thanks for reporting it.
tim-one commented on Apr 27, 2026
Well, I haven't been able to figure out what we intend to support for cookie values containing double quotes. We accept more than JSON encoding can produce - that always backslash-escapes an internal double quote, which makes it easy to check. Our regexp as written can often accept internal double quotes without escaping, which makes it ambiguous whether a double quote is internal or signals the end of a double-quoted string.
Or maybe I'm misunderstanding it. But can't fix it if I don't understand it 😦
KowalskiThomas commented on Apr 27, 2026
Updated the reproducer on my branch to make it way simpler -- initially I wanted it to showcase a "real life" use case with a server/client just to make sure this was an actually realistic scenario, but I think now I have found the tornado and bottle examples anyway, we can probably assume it's realistic.
I'll give the old/new regexes another look later today, I just wanted to report it as soon as possible; the 3.15 release is still pretty far but the sooner it's reported and fixed the better!
tim-one commented on Apr 27, 2026
Agreed on all points - and, if possible, I'd like to get it fixed before the upcoming 3.14 patch release. If 3.14.5 goes out as-is, then this becomes a "security issue" for real.
StanFromIreland commented on Apr 27, 2026
d7dbde8 wasn't backported to 3.14?
tim-one commented on Apr 27, 2026
Ah! Good catch, @StanFromIreland! I really wouldn't know. All I do know is that "it's broken" on the main branch.
To illustrate why I'm baffled about our intent, here on the main branch:
JSON encoding will never produce such a string, and standards don't allow for it either. We're obviously trying to cater to some more-or-less common abuse, but I can't figure out exactly what. Python itself parses
as a catenation of two double-quoted strings, evaluating to plain
The cookie code is stripping off the "enclosing" double quotes and leaving the two in the middle alone. It's ambiguous, and the current regexp relies on a minimal match to break ambiguity (which can lead to exponential time to fail to match an input). JSON would encode it as
which is dead easy to deal with. I assume, but don't know, that the PR adding this stuff intended to deal with JSON encodings showing up in cookie values, but coded a flawed regexp that accepts a much broader set of strings.
picnixz commented on Apr 27, 2026
Indeed, https://www.rfc-editor.org/rfc/rfc6265.html#section-4.1.1 grammar is:
set-cookie-header = "Set-Cookie:" SP set-cookie-string set-cookie-string = cookie-pair *( ";" SP cookie-av ) cookie-pair = cookie-name "=" cookie-value cookie-name = token cookie-value = *cookie-octet / ( DQUOTE *cookie-octet DQUOTE ) cookie-octet = %x21 / %x23-2B / %x2D-3A / %x3C-5B / %x5D-7E ; US-ASCII characters excluding CTLs, ; whitespace DQUOTE, comma, semicolon, ; and backslashHowever, there were some updates (see #92936 (comment)) and the specs seem to allow a bit more things.
cc @Dreamsorcerer @collinanderson @orsenthil
On a personal note, I think SimpleCookie should do one of the following:
KowalskiThomas commented on Apr 27, 2026
I was thinking earlier today about how this could be fixed; the conclusion I reached was that the last option might be the best suited one, as the default behaviour would be safe, and users that do need unsafe patterns (like in the original issue #92936) could "take the risk", but knowingly do so.
tim-one commented on Apr 27, 2026
I'm not a "cookie guy", and am out of my depth here. It appears we've always intended to be "liberal in what we accept", but in undocumented ways. The first part of that is a good thing ;-)
Since it's apparently never been released, we could revert the commit that introduced the ill-considered implementation of double-quote liberalization, and wait for someone to do a more careful job.
Digging through that commit and the issue report it closed, looks like @collinanderson is a bona fide expert who was willing to help at the time, Perhaps they could be persuaded to do so now?
collinanderson commented on Apr 27, 2026
Yeah I'm not a huge expert here, but I did do a deep dive on this 10 years ago. I probably don't have the motivation to do a patch and push it through myself, but I could help with reviews.
I'm a fan of "liberal in what we accept", and like I said on the other thread, the spec has a simple algorithm for permissive parsing in Section 5.2 for the Set-Cookie header, which is generally what browsers do: https://www.rfc-editor.org/rfc/rfc6265.html#section-5.2 It needs to be modified for Cookie header, but basically just split on %x3B (";") and then run each name-value-pair through this:
NOTE: The algorithm below is more permissive than the grammar in Section 4.1. For example, the algorithm strips leading and trailing whitespace from the cookie name and value (but maintains internal whitespace), whereas the grammar in Section 4.1 forbids whitespace in these positions. User agents use this algorithm so as to interoperate with servers that do not follow the recommendations in Section 4. A user agent MUST use an algorithm equivalent to the following algorithm to parse a "set-cookie-string": 1. If the set-cookie-string contains a %x3B (";") character: The name-value-pair string consists of the characters up to, but not including, the first %x3B (";"), and the unparsed- attributes consist of the remainder of the set-cookie-string (including the %x3B (";") in question). Otherwise: The name-value-pair string consists of all the characters contained in the set-cookie-string, and the unparsed- attributes is the empty string. 2. If the name-value-pair string lacks a %x3D ("=") character, ignore the set-cookie-string entirely. 3. The (possibly empty) name string consists of the characters up to, but not including, the first %x3D ("=") character, and the (possibly empty) value string consists of the characters after the first %x3D ("=") character. 4. Remove any leading or trailing WSP characters from the name string and the value string. 5. If the name string is empty, ignore the set-cookie-string entirely. 6. The cookie-name is the name string, and the cookie-value is the value string.Oh: and if I remember right, SimpleCookie has a bit of confusion as to whether we're parsing a Set-Cookie header or a Cookie header. They really need different parsers becauseSet-Cookie: a=b; path=/ means something totally different (1 cookie with a path) than Cookie: a=b; path=/ (two cookies, the 2nd named "path").
tim-one commented on Apr 27, 2026
Thanks, @collinanderson! We don't need help writing code, but would greatly benefit from an experienced developer's view of what our goals should be (rigorously defining the real-world extensions we intend to accept).
In my view, our cookie parsing suffers from "Perl disease": regexps excel at implementing maximal-munch lexers, but create all sorts of problems when code tries to apply them to full-blown parsing. In the old days, people used les+yacc for a proper separation of concerns. The issue at hand is an illustration: the "do everything all at once" regexp can suffer unintended "surprise" exponential-time behavior on inputs that don't match. A maximal-munch lexsr would peel off one token at at time, and never backtrack. Whether or not, e.g., to tread semicolons differently depending on context is then up to explicit logic consuming the token stream, not hiding in the bowels of two mostly duplicated 20-line regexps. Which we don't have now anyway: the same fat regexp is used regardless of cookie contest (which is almost certainly why Set-Cookie and Cookie headers are treated alike).
32 remaining items
serhiy-storchaka commented on Apr 29, 2026
One thing bothers me. All "real world" examples in #92936 and #71861 use non-quoted JSON in cookie. So why the regex for quoted strings was changed? This does not help with that examples.
It is possible to use RFC 2109/RFC 2965 for quoted strings and RFC 6265 for unquoted strings.
Dreamsorcerer commented on Apr 29, 2026
The proposal is to effectively allow anything (except security concerns like CTL chars), so it wouldn't break backwards compatibility. But, yes, this particular path doesn't need to be rushed in for 3.15, it can also be done later. That wasn't my intention.
tim-one commented on Apr 29, 2026
@serhiy-storchaka, yes, the original issue was hard to follow, sloppy about details. Nevertheless, the actual patch obviously targeted only double-quoted strings as a whole. Which the OP acknowledged in a comment. But I'm fine with just reverting the PR, period. It wasn't sufficiently thought out to begin with, and on more than one count.
@Dreamsorcerer
Moving to just splitting on semicolons would break stuff. I've never before used this code, so that even newbie-me realizes that should be suggestive 😉.
For example, this "works" now (3.14):
That is, semicolon loses its privileged status inside a dquted string value now. And we also evaluate some escape sequences in dquoted string values now, like
That one isn't about parsing, but about active transformations of the values. I personally don't think we "should be" in that business either.
I think that best. My take for 3.15: revert the reDOS PR, period.
serhiy-storchaka commented on Apr 29, 2026
See the draft #149145. It uses the hybrid approach.
But there are other issues. If we make parsing too lenient, this can cause security issues. This will break some tests. If we make parsing more strict, what rules should we use? They are different between RFCs. This will break other existing tests. What is worse, handling invalid characters is inconsistent -- some cause an exception, others causes silent ignoring of the cookie.
There are other issues. For example, I suspect that existing non-standard encoding scheme was designed for 8-bit strings in Python 2. It does not work as expected for non-ASCII strings in Python 3.
collinanderson commented on Apr 29, 2026
Too late for 3.15, but long term I suggest either 1. lenient: keeping it simple and following the spec (RFC 6265) and allowing everything (except nameless cookies), and then users can then filter/exclude/reject further, or 2. strict: matching the official web-platform-tests (including allowing nameless cookies). (The spec should probably get updated to match web-platform-tests anyway)
https://github.com/web-platform-tests/wpt/blob/master/cookies/name/name-ctl.html
https://github.com/web-platform-tests/wpt/blob/master/cookies/value/value-ctl.html
https://github.com/web-platform-tests/wpt/blob/master/cookies/name/name.html
https://github.com/web-platform-tests/wpt/blob/master/cookies/value/value.html
https://github.com/web-platform-tests/wpt/blob/master/cookies/size/name-and-value.html
etc
I think browsers encode the cookie header as cp1252, which generally limits the possible incoming characters.
tim-one commented on Apr 29, 2026
Unclear to me what "hybrid approach" means to you, exactly. This line in the PR is troublesome:
The first alternative can consume a backslash. But so can the second alternative. The match engine can't guess which to use in advance, so potentially piles up an unbounded sequence of 2-way branches to explore. I understand that the possessive quantifier prevents actually backtracking, but a robust regexp really shouldn't rely on that. Best practice is to ensure all "choice points" are mutually exclusive at the time they're first seen.
That's easily achieved here by changing the second alternative to [^"\\] You never intended for the second alternative to match a backslash anyway. Be explicit about that?
I assume you realize this also gives up on what the reDOS PR intended to accomplish (allow unescaped dquotes in a string value too).
I believe that's right. There's every sign that cookies.py implicitly assumes the character set is confined to 7-bit ASCII.
But this particular issue is really unsuited to discussing such fundamental changes.
tim-one commented on Apr 29, 2026
Another caution about haste 😉. "My" regexp tweak is solid so far as it goes, but it's also flawed.
yields the hoped-for
but the variant
yields the unintended
That the raw dquote is followed by a (optional string of whitespace followed by a) semicolon prevents the "very careful" regexp from accepting the raw dquote, assuming \s*; (or \s*$) following it must be the end of the cookie key=value pair input. So that dquote is not consumed in the guts, but is taken instead to be the closing dquote of the string, and the semicolon following it taken to mean "end of input field".
This is typical of mysteries that pop up when trying to use regexps for "real parsing". They're often inadequate to the full task without heroic effort.
Compounded by that we just let failed parses pass silently,. In the variation, it leaves the lone raw dquote at the end untouched, which isn't matched by anything, but we don't tell the user that we couldn't make any sense of that. But when using regexps for parsing, it's hard too to create a useful error message. "CooikeError: internal do-everything-all-at-once regexp failed to match" leaves a bit to be desired 😉.
tim-one commented on Apr 30, 2026
To drive home why "quoted value string can contain anything" is intractable, here's an input:
There are then two different ways to parse it. Either it's setting x to
or it's setting two keys
We can't know whether a semicolon does or doesn't terminate an input field unless we know whether it appears inside or outside a quoted string. But we can't know whether an unescaped dquote terminates a string unless we know whether it's the last significant character in an input field. Combined, those leave the "; substring of the input hopelessly ambiguous.
Which is why every other gimmick on Earth establishes rigid rules for how to spell "here this meta-character has no special meaning - it's just another character".
Why this doesn't bite us now (3.14): inside a quoted string, semicolon has no meta-meaning, period. But an unescaped dquote always has its meta-meaning. The string ends when and only when we encounter an unsscaped dqupte. So ";, unless preceded by backslash (escaping the dquote) means "the string ended, and so did the input field". The semicolon regains its meta-meaning.
"My regexp" aims to preserve that resolution, tying the interpretation of a raw dquote to whether it is or isn't followed by \s*(;|$). Ugly, though - it's not the characters on their own that drive it, but rather how they combine in context.
serhiy-storchaka commented on Apr 30, 2026
Since my PR completely overwrites the previous changes, I think that it will be clearer if just revert them first.
It does not match any specification and does not solve the original issue. It is also inherently ambiguous.
orsenthil commented on Apr 30, 2026
I think, this is good to revert the change right now and discuss separately if it is worth fixing, given the disadvantages.
gh-149028: Revert gh-92936 changes (GH-149182)
tim-one commented on Apr 30, 2026
Since reverting the reDOS PRs is complete:
#149182
I'm closing this issue report. Other ideas for reworking cookies.py are most welcome, but should be pursued elsewhere (e.g., in the "Ideas" category of discuss.python.org).