[security] directory traversal in tempfile prefix

[YusukeEndoh]

BPO	35278
Nosy	@mjpieters, @vstinner, @ambv, @csabella, @Thorleon, @obestwalter
PRs	bpo-35278: Sanitize tempfile prefix to prevent directory treversal #10627
Files	bpo-35278.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 = None
closed_at = None
created_at = <Date 2018-11-19.12:46:03.177>
labels = ['type-security', '3.8', '3.9', '3.10', '3.7', 'library']
title = '[security] directory traversal in tempfile prefix'
updated_at = <Date 2021-03-14.02:25:00.048>
user = 'https://bugs.python.org/YusukeEndoh'

bugs.python.org fields:

activity = <Date 2021-03-14.02:25:00.048>
actor = 'gregory.p.smith'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Library (Lib)']
creation = <Date 2018-11-19.12:46:03.177>
creator = 'Yusuke Endoh'
dependencies = []
files = ['47939']
hgrepos = []
issue_num = 35278
keywords = ['patch']
message_count = 6.0
messages = ['330097', '330100', '330169', '335174', '340205', '356299']
nosy_count = 7.0
nosy_names = ['mjpieters', 'vstinner', 'lukasz.langa', 'cheryl.sabella', 'Yusuke Endoh', 'thorleon', 'obestwalter']
pr_nums = ['10627']
priority = 'normal'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue35278'
versions = ['Python 3.6', 'Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10']

[YusukeEndoh]

Hello,

The tempfile library does not check the prefix argument, which can be exploited to create files outside tmpdir by using directory traversal.

>>> import tempfile
>>> tempfile.gettempprefix()
'tmp'
>>> f = tempfile.NamedTemporaryFile(prefix="/home/mame/cracked")
>>> f.name
'/home/mame/crackedlt3y_ddm'

The same issue was found and treated as a vulnerability in PHP (CVE-2006-1494) and Ruby (CVE-2018-6914).

I first reported this issue to security@python.org at July 2018. Some people kindly discussed it, and finally I was told to create a ticket here.

[vstinner]

Ruby handled this issue as a vulnerability:
https://www.ruby-lang.org/en/news/2018/03/28/unintentional-file-and-directory-creation-with-directory-traversal-cve-2018-6914/

The doc of "gettempprefix" says "This does not contain the directory component", so it is natural for users to think "prefix" will accept only a file name.

Maybe we can silently truncated the directort part of the prefix to only keep the base name in stable branches, but raise an exception in Python 3.8? Or maybe emit a deprecation warning in Python 3.7?

[Thorleon]

Hello,
I have created patch and MR for the Python 3.8 "exception" approach.

For the reference here is patch for ruby:
ruby/ruby@e9ddf2b

Maybe we should consider also validation on suffix as in their solution?

[csabella]

Adding Łukasz to the nosy list as release manager.

[obestwalter]

I am not sure if this justifies a new issue so I add this here.

The suffix parameter can also be used for a traversal attack. It is possible to completely clobber anything in dir and prefix (at least on Windows).

e.g. calling mkdtemp or NamedTemporaryFile with these paramers ...

dir=r"C:\tmp",
prefix="pre",
suffix="../../../../../../../../../gotcha"

Will result in a directory or file being created at C:/gotcha.

I also wonder if this would justify adding a warning to the documentation for all existing Python versions?

Quoting from the documentation of mkstemp (https://docs.python.org/3/library/tempfile.html#tempfile.mkstemp):

If prefix is specified, the file name will begin with that prefix; otherwise, a default prefix is used.

If dir is specified, the file will be created in that directory [...]

As both claims are rendered untrue when using suffix in the above described way I think this should be amended.

[mjpieters]

I found this issue after helping someone solve a Stack Overflow question at https://stackoverflow.com/q/58767241/100297; they eventually figured out that their prefix was a path, not a path element.

I'd be all in favour of making tempfile._sanitize_params either reject a prefix or suffix with os.sep or os.altsep characters, or just take the last element of os.path.split().

[xiaoge1001]

Is the problem planned to be solved? I found no reply for a long time.

[vstinner]

So far, nobody proposed a pull request to fix the issue.