Arbitrary code execution vulnerability due to unchecked eval() call in dumbdbm module

[StephenFarris]

BPO	22885
Nosy	@malemburg, @gvanrossum, @rhettinger, @vstinner, @bitdancer, @PCManticore, @serhiy-storchaka, @gvanrossum
Files	issue22885.patch issue22885_1.patch

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2015-02-15.22:40:44.914>
created_at = <Date 2014-11-16.18:39:00.954>
labels = ['type-security', 'library']
title = 'Arbitrary code execution vulnerability due to unchecked eval() call in dumbdbm module'
updated_at = <Date 2015-02-22.16:12:27.930>
user = 'https://bugs.python.org/stephenfarris'

bugs.python.org fields:

activity = <Date 2015-02-22.16:12:27.930>
actor = 'Arfrever'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2015-02-15.22:40:44.914>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2014-11-16.18:39:00.954>
creator = 'stephen.farris'
dependencies = []
files = ['37812', '37813']
hgrepos = []
issue_num = 22885
keywords = ['patch']
message_count = 11.0
messages = ['231255', '234446', '234447', '234450', '234547', '234551', '234598', '234599', '234600', '236073', '236075']
nosy_count = 11.0
nosy_names = ['lemburg', 'gvanrossum', 'rhettinger', 'vstinner', 'Arfrever', 'r.david.murray', 'Claudiu.Popa', 'python-dev', 'serhiy.storchaka', 'Guido.van.Rossum', 'stephen.farris']
pr_nums = []
priority = 'high'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue22885'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5']

[StephenFarris]

The dumbdbm module uses an unchecked call to eval() in the _update method, which is called in response to a call to dumbdbm.open(), and is used to load the index from the directory file.  This poses a security vulnerability because it allows an attacker to execute arbitrary code on the victim's machine by inserting python code into the DBM directory file.  This vulnerability could allow an attacker to execute arbitrary commands on the victim machine, potentially allowing them to deploy malware, gain system access, destroy files and data, expose sensitive information, etc.

[PCManticore]

Here's a patch which uses ast.literal_eval instead. This doesn't get code executed, since literal_eval will fail loudly for anything other than a literal. There are some issues to consider:

• let the current ast.literal_eval call bubble out with a lot of different exceptions
• normalize the exception to dbm.dumb.error.

I'm leaning towards the first, since it clearly shows that something bad happened in the module and it's a first indicator that someone tampered with the data file.

[gvanrossum]

Python 3's exception chaining allows us to do the second (easier to catch without resorting to "except Exception:" or even "except:") while still showing the original exception in the traceback.

[PCManticore]

Thanks for the tip, Guido. The new patch uses exception chaining. If this needs backporting, most probably the first patch can be used.

[vstinner]

+ with open(_fname + ".dir", 'w') as stream:
+ stream.write(content)

I don't see where the created file is deleted. Add something like:

self.addCleanup(support.unlink, _fname + ".dir")

[PCManticore]

Thanks, Victor. I thought the same thing, but the file is deleted here already, here: https://hg.python.org/cpython/file/981ba93bcbde/Lib/test/test_dbm_dumb.py#l228

[serhiy-storchaka]

Raising dbm.dumb.error is behavior change. It would be safer not apply this part in maintained releases.

If add sanity checks in 3.5, note that following line "key = key.encode('Latin-1')" can raise an exception too (AttributeError or UnicodeEncodeError). And incorrect data can cause an error later in __getitem__ if pos_and_siz_pair is not a pair of two integers.

I think it is worth to split this issue on two issues and fix only security issue here.

[PCManticore]

Thanks, Serhiy. Only the security issue is fixed in this patch, since eval is replaced by ast.literal_eval and nothing else. Indeed, if the data is something else than what dbm expects after ast.literal_eval, then it would fail, but it would have failed previously too.

[serhiy-storchaka]

I mean that raising dbm.dumb.error is different issue unrelated to changing eval to ast.literal_eval. See also Raymond's objections in bpo-21708.

bpo-22885.patch LGTM.

[python-dev]

New changeset 02865d22a98d by Serhiy Storchaka in branch '2.7':
Issue bpo-22885: Fixed arbitrary code execution vulnerability in the dumbdbm
https://hg.python.org/cpython/rev/02865d22a98d

New changeset 693bf15b4314 by Serhiy Storchaka in branch '3.4':
Issue bpo-22885: Fixed arbitrary code execution vulnerability in the dbm.dumb
https://hg.python.org/cpython/rev/693bf15b4314

New changeset cf6a62b0ef3b by Serhiy Storchaka in branch 'default':
Issue bpo-22885: Fixed arbitrary code execution vulnerability in the dbm.dumb
https://hg.python.org/cpython/rev/cf6a62b0ef3b

[serhiy-storchaka]

Committed bpo-22885.patch with modified test which demonstrates vulnerability of unpatched dbm.dumb. If you want to change exception type raised by dbm.dumb, you can open new issue.