Python's parse_qsl treated semicolons as separators, enabling web cache poisoning

Python 3.9.1 was the last release that turned a query string into parameters like this:

pairs = [s2 for s1 in qs.split('&') for s2 in s1.split(';')]

Read it twice. Split on &, then split each of those pieces on ;, then flatten. Both characters separate parameters. So a=1&b=2 gives you two parameters, a=1;b=2 gives you two parameters, and a=1;b=2&c=3 gives you three.

There's nothing wrong with that code on its own. parse_qsl does what its docstring promises. It does it efficiently, and it had done it that way for years. The bug is entirely in the gap between what Python believes and what the machine in front of Python believes.

Adam Goldschmidt, of Snyk, mailed the Python Security Response Team about it on 19 October 2020. He filed the public issue three months later. It opens with one sentence:

The urlparse module treats semicolon as a separator [...] whereas most proxies today only take ampersands as separators.

Parameter cloaking#

A caching proxy sits in front of your app. It remembers responses so it doesn't have to ask you again. To do that it needs a cache key, a string it can use to decide whether two requests are "the same request". The obvious key is the method plus the host plus the full URL. Use that and everything is correct. Your cache hit rate is also terrible.

So real caches trim the key. They know that some query parameters change the response and some don't. ?page=3 matters. ?utm_source=newsletter is analytics tracking that the application ignores, so caches are routinely configured to strip it out of the key. Parameters excluded from the key are usually called unkeyed.

The attack has a name: parameter cloaking. Suppose the cache is told to drop utm_source from the key, and the application has some parameter that changes the response body. An attacker requests:

GET /article?utm_source=x;callback=whatever HTTP/1.1

The cache splits on &. It finds exactly one parameter named utm_source, whose value happens to be x;callback=whatever. That parameter is unkeyed, so the computed key is equivalent to plain /article. Python splits on & and ;. It finds two parameters, and hands the application a callback it was never supposed to see from this URL.

The application generates a different response. The cache stores that response under the key for the clean, ordinary /article. From then on it serves that response to everyone who asks for the article.

The advisory says the same thing more carefully. An attacker can cause "a difference in the interpretation of the request between the proxy (running with default configuration) and the server". That difference can result in "malicious requests being cached as completely safe ones". The reason is that "the proxy would usually not see the semicolon as a separator, and therefore would not include it in a cache key of an unkeyed parameter."

Neither side is malfunctioning. The proxy is right, by the standards of every other HTTP stack in the building. Python is right too, by the standards of whatever it was reading when that line got written. An attacker doesn't need a bug in either one. He needs them to be right about different things.

What the patch changes#

The patched parse_qsl splits on one string, and the caller gets to say which:

def parse_qsl(qs, keep_blank_values=False, strict_parsing=False,
              encoding='utf-8', errors='replace', max_num_fields=None, separator='&'):
    ...
    if not separator or (not isinstance(separator, (str, bytes))):
        raise ValueError("Separator must be of type string or bytes.")
    ...
    pairs = [s1 for s1 in qs.split(separator)]

Two changes worth pointing at. The default is now & alone, which is the actual behaviour change. And separator is a real parameter. Anyone who wanted the old semicolon handling can ask for it, one call site at a time, in the open, where a reviewer can see it. That's a much better position than a global default nobody knew about.

Note also max_num_fields. It used to count qs.count('&') + qs.count(';') and now counts occurrences of separator. That's the kind of thing you'd miss on a first pass. Change the split and leave the count alone, and the field limit would be computed against a different notion of "field" than the parse. That is exactly the class of mismatch the whole CVE is about.

This is CVE-2021-23336, tracked upstream as bpo-42967.

Dates and rollout#

Reported 19 October 2020. The public issue was opened 19 January 2021, three months later. The fixes landed across branches on 14 and 15 February 2021. The releases followed within days: 3.6.13 and 3.7.10 on 16 February, 3.8.8 and 3.9.2 on 19 February. 3.10 got it in 3.10.0.

Three months of quiet is not unusual here, because the change is not really a bug fix. It is a deliberate, documented alteration to what a widely used standard library function returns. It shipped in patch releases across four maintained branches at once, plus main. Every web framework that calls parse_qsl gets the new behaviour. So does every WSGI app that leans on cgi.parse underneath, and every internal tool that parses a callback URL. It arrives on a pip install --upgrade or a distro security update, with no code change and no warning.

You can see the maintainers bracing for that in what the distributors shipped. Red Hat's backport does more than change the default and walk away. It gives administrators a way to put the semicolon back without touching application code, through configuration files, an environment variable, or a call in Python. That knob doesn't exist because everyone thought the transition would be smooth.

The rest of the patch is the same edit repeated across five branches. 5c17dfc for 3.6, d0d4d30 for 3.7, e3110c3 for 3.8, c9f0781 for 3.9, fcbe0cb for 3.10.

One last thing, not a security matter. It is my favourite line in parse.py, and it survived the patch untouched. The docstring for parse_qsl ends:

Returns a list, as G-d intended.