[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[STINNER Victor]

Change by STINNER Victor :


--
nosy:  -vstinner

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Éric Araujo]

Éric Araujo  added the comment:

See the changelog entry for 2021-11-04 10:31:24 (and the other ticket where 
Guido just commented)

(and thanks for cleaning spam!)

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

See bpo-12168 for a similar cleanup by Eryk Sun. There was approx. 20 spammed 
issues. Eryk fixed most of them; I did a couple.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Erlend E. Aasland]

Erlend E. Aasland  added the comment:

Yes, cleaning up ahmedsayeed1982 spam. I did my best to revert the nosy list, 
component, versions, and assigned to changes. What did I mess up?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Éric Araujo]

Éric Araujo  added the comment:

erlandaasland you’ve been editing closed issues today (got messages from at 
least 2).  maybe submitting old browser tabs with obsolete form data?

--
nosy: +erlendaasland

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
nosy: +AdamGold, eric.araujo, gregory.p.smith, kj, lemburg, mcepl, 
miss-islington, ned.deily, orsenthil, pablogsal, petr.viktorin, rschiron, 
serhiy.storchaka, vstinner -ahmedsayeed1982
versions: +Python 3.10, Python 3.6, Python 3.7, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Erlend E. Aasland]

Change by Erlend E. Aasland :


--
Removed message: https://bugs.python.org/msg405709

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Ahmed Sayeed]

Ahmed Sayeed  added the comment:

[gdb/symtab] Handle DW_TAG_type_unit in process_psymtab_comp_unit 

When running test-case gdb.cp/cpexprs-debug-types.exp with target board
unix/gdb:debug_flags=-gdwarf-5, I run into:
...
(gdb) file cpexprs-debug-types^M
Reading symbols from cpexprs-debug-types...^M
ERROR: Couldn't load cpexprs-debug-types into GDB (eof).
ERROR: Couldn't send delete breakpoints to GDB.
ERROR: GDB process no longer exists
GDB process exited with wait status 23054 exp9 0 0 CHILDKILLED SIGABRT 
SIGABRT
... https://www.webb-dev.co.uk/crypto/crypto-for-investing/

We're running into this abort in process_psymtab_comp_unit:
...
  switch (reader.comp_unit_die->tag) 
{
case DW_TAG_compile_unit:
  this_cu->unit_type = DW_UT_compile; 
http://www.compilatori.com/tech/nvidia-and-samsung/
  break;
case DW_TAG_partial_unit:
  this_cu->unit_type = DW_UT_partial; 
http://www.acpirateradio.co.uk/tech/nvidia-and-samsung/
  break;
default:
  abort (); http://www.logoarts.co.uk/tech/nvidia-and-samsung/ 
}
...
because reader.comp_unit_die->tag == DW_TAG_type_unit.
 http://www.slipstone.co.uk/tech/nvidia-and-samsung/
Fix this by adding a DW_TAG_type_unit case.

Tested on x86_64-linux.

gdb/ChangeLog: http://embermanchester.uk/tech/nvidia-and-samsung/
[gdb/symtab] Handle DW_TAG_type_unit in process_psymtab_comp_unit

When running test-case gdb.cp/cpexprs-debug-types.exp with target board
unix/gdb:debug_flags=-gdwarf-5, I run into: 
http://connstr.net/tech/nvidia-and-samsung/
...
(gdb) file cpexprs-debug-types^M
Reading symbols from cpexprs-debug-types...^M 
http://joerg.li/tech/nvidia-and-samsung/
ERROR: Couldn't load cpexprs-debug-types into GDB (eof).
ERROR: Couldn't send delete breakpoints to GDB.
ERROR: GDB process no longer exists 
http://www.jopspeech.com/tech/nvidia-and-samsung/
GDB process exited with wait status 23054 exp9 0 0 CHILDKILLED SIGABRT 
SIGABRT
...

We're running into this abort in process_psymtab_comp_unit: 
http://www.wearelondonmade.com/tech/nvidia-and-samsung/
...
  switch (reader.comp_unit_die->tag)
{
case DW_TAG_compile_unit: 
https://waytowhatsnext.com/technology/korean-technology/
  this_cu->unit_type = DW_UT_compile;
  break;
case DW_TAG_partial_unit: 
http://www.iu-bloomington.com/technology/miui13/
  this_cu->unit_type = DW_UT_partial;
  break;
default:
  abort (); https://komiya-dental.com/technology/miui-13/
}
...
because reader.comp_unit_die->tag == DW_TAG_type_unit.
http://www-look-4.com/tech/nvidia-and-samsung/
Fix this by adding a DW_TAG_type_unit case.

Tested on x86_64-linux.
https://www.webb-dev.co.uk/technology/grt-r910/
gdb/ChangeLog:

--
nosy: +ahmedsayeed1982 -AdamGold, eric.araujo, gregory.p.smith, kj, lemburg, 
mcepl, miss-islington, ned.deily, orsenthil, petr.viktorin, rschiron, 
serhiy.storchaka, vstinner
versions:  -Python 3.10, Python 3.6, Python 3.7, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Senthil Kumaran]

Senthil Kumaran  added the comment:


New changeset d5b80eb11b4812b4a579ce129ba4a10c5f5d27f6 by Miss Islington (bot) 
in branch '3.8':
bpo-42967: coerce bytes separator to string in urllib.parse_qs(l) (GH-24818) 
(#25345)
https://github.com/python/cpython/commit/d5b80eb11b4812b4a579ce129ba4a10c5f5d27f6


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Matej Cepl]

Matej Cepl  added the comment:

> Did you upstream fixes for those packages?

Of course we did. Upstream first!

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[miss-islington]

miss-islington  added the comment:


New changeset 6ec2fb42f93660810952388e5c4018c197c17c8c by Miss Islington (bot) 
in branch '3.9':
bpo-42967: coerce bytes separator to string in urllib.parse_qs(l) (GH-24818)
https://github.com/python/cpython/commit/6ec2fb42f93660810952388e5c4018c197c17c8c


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[miss-islington]

Change by miss-islington :


--
pull_requests: +24079
pull_request: https://github.com/python/cpython/pull/25345

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 13.0 -> 14.0
pull_requests: +24078
pull_request: https://github.com/python/cpython/pull/25344

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Senthil Kumaran]

Senthil Kumaran  added the comment:


New changeset b38601d49675d90e1ee6faa47f7adaeca992d02d by Ken Jin in branch 
'master':
bpo-42967: coerce bytes separator to string in urllib.parse_qs(l) (#24818)
https://github.com/python/cpython/commit/b38601d49675d90e1ee6faa47f7adaeca992d02d


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Senthil Kumaran]

Senthil Kumaran  added the comment:

Petr, 

On 

> the `separator` argument now allows multi-character strings, so you can parse 
> 'a=1b=2' with separator=''. Was this intentional?

No, this was not intentional. The separator arg was just coice, for  
compatibility, if some wanted to use `;` like the some URLs that were shared as 
use case. We didn't restrict about what was allowed or length of the separator.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Petr Viktorin]

Petr Viktorin  added the comment:

There's another part of the new implementation that looks a bit fishy: the 
`separator` argument now allows multi-character strings, so you can parse 
'a=1b=2' with separator=''.
Was this intentional?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Gregory P. Smith]

Gregory P. Smith  added the comment:

Riccardo - FWIW I agree, the wrong part of the stack was blamed and a CVE was 
wrongly sought for against CPython on this one.

It's sewage under the bridge at this point. The API change has shipped in 
several different stable releases and thus is something virtually Python all 
code must now deal with.

Why was this a bad change to make?  Python's parse_qsl obeyed the prevailing 
HTML 4 standard at the time it was written:

https://www.w3.org/TR/html401/appendix/notes.html#ampersands-in-uris

'''
We recommend that HTTP server implementors, and in particular, CGI implementors 
support the use of ";" in place of "&"
'''

That turns out to have been bad advice in the standard. 15 years later the 
html5 standard quoted in Adam's snyk blog post links to its text on this which 
leaves no room for that interpretation.

In that light, the correct thing to do for this issue would be to:

* Make the default behavior change in 3.10 match the html5 standard [done].
* Document that it matches the html4 standard in 3.9 and earlier without 
changing their default behavior [oops, too late, not done].
* While adding the ability to allow applications to select the stricter 
behavior on those older versions.  [only sort of done, and somewhat too late 
now that the strict version has already shipped as stable]

Afterall, the existence of html5 didn't magically fix all of the html and web 
applications written in the two decades of web that came before it.  Ask any 
browser author...

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Riccardo Schirone]

Riccardo Schirone  added the comment:

> So far, we at openSUSE had to package at least SQLAlchemy, Twisted, yarl and 
> furl. The author of the first one acknowledged use of semicolon as a bug. I 
> don't think it was so bad.

Did you upstream fixes for those packages?

Asking because if this is considered a vulnerability in Python, it should be 
considered a vulnerability for every other tool/library that accept `;` as 
separator. For example, Twisted seems to have a parse_qs method in web/http.py 
file that splits by both `;` and `&`.

Again, I feel like we are blaming the wrong piece of the stack, unless proxies 
are usually ignoring some arguments (e.g. utm_*) as part of the cache key, by 
default or in a very easy way.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Ken Jin]

Change by Ken Jin :


--
pull_requests: +23584
pull_request: https://github.com/python/cpython/pull/24818

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Senthil Kumaran]

Senthil Kumaran  added the comment:

Petr, thank you. Let's treat it as a new issue linked to this.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Petr Viktorin]

Petr Viktorin  added the comment:

With the fix, parse_qs[l] doesn't handle bytes separators correctly.
There is an explicit type check for str/bytes:

if not separator or (not isinstance(separator, (str, bytes))):
raise ValueError("Separator must be of type string or bytes.")

but a bytes separator fails further down:

>>> import urllib.parse
>>> urllib.parse.parse_qs('a=1,b=2', separator=b',')
Traceback (most recent call last):
  File "", line 1, in 
  File "/home/pviktori/dev/cpython/Lib/urllib/parse.py", line 695, in parse_qs
pairs = parse_qsl(qs, keep_blank_values, strict_parsing,
  File "/home/pviktori/dev/cpython/Lib/urllib/parse.py", line 748, in parse_qsl
pairs = [s1 for s1 in qs.split(separator)]
TypeError: must be str or None, not bytes

--
nosy: +petr.viktorin

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Riccardo Schirone]

Riccardo Schirone  added the comment:

This CVE was reported against Python, however it does not seem to be Python's 
fault for supporting the `;` separator, which was a valid separator for older 
standards.

@AdamGold for this issue to become a real security problem, it seems that the 
proxy has to be configured to ignore certain parameters in the query. For NGINX 
and Varnish proxies mentioned in the article it seems that by default they use 
the entire request path, host included, and other things as cache key. For 
NGINX in particular I could find some snippets online to manipulate the query 
arguments and split them in arguments, so to remove the "utm_*" arguments, 
however this does not seem a standard(or at least default) behaviour, nor 
something easily supported.

I think that if that is the case and a user has to go out of his way to 
configure the (wrong) splitting of arguments in the proxy, it is not fair to 
blame python for accepting `;` as separator and assigning a CVE against it may 
cause confusion.

For distributions this is problematic as they have 2 choices:
1) "fix" python but with the risk of breaking user's programs/scripts relying 
on the previous API
2) keep older version/unpatched python so that user's programs still work, but 
with a python version "vulnerable" to this CVE.

None of these options is really ideal, especially if the problem is somewhere 
else.

@AdamGold Could you elaborate a bit more on how common it is and how much 
configuration is required for proxies to make `;` a problem in python?

--
nosy: +rschiron

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Gregory P. Smith]

Gregory P. Smith  added the comment:

An example code snippet to detect if the API supports the new parameter at 
runtime for code that wants to use to use something other than the default '&'.

```
if 'separator' in inspect.signature(urllib.parse.parse_qs).parameters:
... parse_qs(..., separator=';')
else:
... parse_qs(...)
```

calling it with the arg and catching TypeError if that fails would also work, 
but might not be preferred as catching things like TypeError is non-specific 
and could hide other problems, making it a code maintenance headache.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Matej Cepl]

Matej Cepl  added the comment:

Port of the patch to 2.7.18.

--
Added file: 
https://bugs.python.org/file49839/CVE-2021-23336-only-amp-as-query-sep.patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Matej Cepl]

Matej Cepl  added the comment:

> FYI - This was somewhat of an unfortuate API change.  I'm coming across code 
> that relies on ; also being treated as a separator by parse_qs().  That code 
> is now broken with no easy way around it.

So far, we at openSUSE had to package at least SQLAlchemy, Twisted, yarl and 
furl. The author of the first one acknowledged use of semicolon as a bug. I 
don't think it was so bad.

--
nosy: +mcepl

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Gregory P. Smith]

Gregory P. Smith  added the comment:

FYI - This was somewhat of an unfortuate API change.  I'm coming across code 
that relies on ; also being treated as a separator by parse_qs().  That code is 
now broken with no easy way around it.

And I'm only seeing things lucky enough to have an explicit test that happens 
to rely in some way on that behavior.  How much code doesn't?

It's been a mix of some clearly broken code (ex  appearing in the URI 
being parsed) and code where it is not so immediately obvious if there is a 
problem or not (up to the code owners to dive in and figure that out...).

The workarounds for people implementing "fixes" to previously working as 
intended rather than "oops that was a html charref" code are annoying.  Our new 
separator= parameter does not allow one to achieve the previous behavior if 
mixing and matching & And ; was intended to be allowed, as it is a single 
separator rather than a set of separators.

For security fixes, a way for people to explicitly opt-in to 
now-deemed-undesirable-by-default behavior they got from the API is desirable.  
We failed to provide that here.

Just a heads up with no suggested remediation for now.  I'm still unsure how 
big a problem this will turn out to be or not or if it is identifying actual 
worthwhile issues in code.  It's certainly a headache for a few.

--
nosy: +gregory.p.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[STINNER Victor]

STINNER Victor  added the comment:

I created 
https://python-security.readthedocs.io/vuln/urllib-query-string-semicolon-separator.html
 to track fixes of this vulnerability.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue42967] [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a query args separator

[Senthil Kumaran]

Senthil Kumaran  added the comment:

This is resolved in all version of Python now. 
Thank you all for your contributions!

--
resolution:  -> fixed
stage: patch review -> resolved
status: open -> closed
title: [security] urllib.parse.parse_qsl(): Web cache poisoning - `; ` as a 
query args separator -> [CVE-2021-23336] urllib.parse.parse_qsl(): Web cache 
poisoning - `; ` as a query args separator

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com