classification
Title: gettext: DoS via crafted Plural-Forms
Type: security Stage: resolved
Components: Library (Lib) Versions: Python 3.7, Python 3.6, Python 3.4, Python 3.5, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder: Arbitrary code execution in gettext.c2py
View: 28563
Assigned To: Nosy List: barry, benjamin.peterson, christian.heimes, jwilk, pitrou, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2013-06-27 22:01 by jwilk, last changed 2016-11-08 19:32 by serhiy.storchaka. This issue is now closed.

Files
File name Uploaded Description Edit
testcase.mo jwilk, 2013-06-27 22:01
testcase.py jwilk, 2013-06-27 22:01
18317_gettext.patch christian.heimes, 2013-06-28 08:48 review
18317_gettext2.patch christian.heimes, 2013-06-28 14:57 review
Messages (9)
msg191963 - (view) Author: Jakub Wilk (jwilk) Date: 2013-06-27 22:01
It is possible to craft a MO file with Plural-Forms taking arbitrary amounts of CPU and memory to evaluate. A test case is attached.

I realize that opening unstrusted MO files is a rather unusual use case, but the module already contains some code to protect againt malicious Plural-Forms, so I thought you might want to fix this problem as well.
msg191968 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-06-27 23:01
Thanks,

can you please provide the PO file, too? Or did you construct the MO file manually?
msg191969 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-06-27 23:10
Ah, I see what you are doing. Nice catch!

Plural-Forms: nplurals=0; plural=42**42**42;

The plural form gets parsed by gettext.c2py() and eventually turned into a lambda that executes int(42**42**42). Perhaps a custom AST visitor could be used to filter out dangerous ops and limit the amount of ops to a sane amount?
msg191972 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2013-06-28 00:41
Why do we have "support" for untrusted MO files?
msg191980 - (view) Author: Antoine Pitrou (pitrou) * (Python committer) Date: 2013-06-28 08:10
I would rather ask: why do we eval() MO files?
msg191981 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-06-28 08:48
We don't eval() the whole MO file. It's just the pluralization formula, http://www.gnu.org/software/gettext/manual/gettext.html#index-nplurals_0040r_007b_002c-in-a-PO-file-header_007d-1093

The patch uses ast.NodeVisitor to look for dangerous code.
msg191983 - (view) Author: Jakub Wilk (jwilk) Date: 2013-06-28 09:27
Making token filtering more thorough may be simpler that going through AST.

I think Python should accept all the operators that GNU gettext accepts:
http://git.savannah.gnu.org/cgit/gettext.git/tree/gettext-runtime/intl/plural.y?id=v0.18.2.1#n132
msg191995 - (view) Author: Christian Heimes (christian.heimes) * (Python committer) Date: 2013-06-28 14:57
Thanks for the link plural.y! I was looking for a C file, not a YACC file.

The AST approach has advantages over tokenizing. The tokenizer returns just symbols but the AST has also context information. It makes it much easier to distinguish between unary - and binary -. Gettext supports substraction but doesn't allow negative numbers.

Python's gettext is not as strict as GNU gettext. For 3.4 I like to forbid oct and hex numbers, too.
msg280338 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2016-11-08 19:32
The DoS as well as other flaws is fixed in issue28563 by implementing a complete parser for GNU gettext plural form expressions.
History
Date User Action Args
2016-11-08 19:32:58serhiy.storchakasetstatus: open -> closed

superseder: Arbitrary code execution in gettext.c2py

nosy: + serhiy.storchaka
messages: + msg280338
resolution: fixed
stage: test needed -> resolved
2016-09-08 23:44:11christian.heimessetversions: + Python 3.5, Python 3.6, Python 3.7, - Python 3.2, Python 3.3
2013-06-28 14:57:52christian.heimessetfiles: + 18317_gettext2.patch

messages: + msg191995
2013-06-28 09:27:23jwilksetmessages: + msg191983
2013-06-28 08:48:11christian.heimessetfiles: + 18317_gettext.patch
keywords: + patch
messages: + msg191981
2013-06-28 08:10:35pitrousetmessages: + msg191980
2013-06-28 00:41:41benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg191972
2013-06-27 23:10:57christian.heimessetnosy: + barry, - loewis
messages: + msg191969
2013-06-27 23:02:46christian.heimessetnosy: + loewis, pitrou
2013-06-27 23:01:28christian.heimessetversions: + Python 2.7, Python 3.2, Python 3.3, Python 3.4
nosy: + christian.heimes

messages: + msg191968

stage: test needed
2013-06-27 22:01:32jwilksetfiles: + testcase.py
2013-06-27 22:01:26jwilkcreate