classification
Title: [security] directory traversal in tempfile prefix
Type: security Stage: patch review
Components: Library (Lib) Versions: Python 3.8
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Yusuke Endoh, cheryl.sabella, lukasz.langa, mjpieters, obestwalter, thorleon, vstinner
Priority: normal Keywords: patch

Created on 2018-11-19 12:46 by Yusuke Endoh, last changed 2019-11-09 13:09 by mjpieters.

Files
File name Uploaded Description Edit
bpo-35278.patch thorleon, 2018-11-21 01:35
Pull Requests
URL Status Linked Edit
PR 10627 open python-dev, 2018-11-21 01:20
Messages (6)
msg330097 - (view) Author: Yusuke Endoh (Yusuke Endoh) Date: 2018-11-19 12:46
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.
msg330100 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018-11-19 14:05
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?
msg330169 - (view) Author: Tomasz Jezierski (thorleon) * Date: 2018-11-21 01:35
Hello,
I have created patch and MR for the Python 3.8 "exception" approach.

For the reference here is patch for ruby:
https://github.com/ruby/ruby/commit/e9ddf2ba41a0bffe1047e33576affd48808c5d0b

Maybe we should consider also validation on suffix as in their solution?
msg335174 - (view) Author: Cheryl Sabella (cheryl.sabella) * (Python committer) Date: 2019-02-10 22:15
Adding Łukasz to the nosy list as release manager.
msg340205 - (view) Author: Oliver Bestwalter (obestwalter) Date: 2019-04-14 12:35
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.
msg356299 - (view) Author: Martijn Pieters (mjpieters) * Date: 2019-11-09 13:09
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().
History
Date User Action Args
2019-11-09 13:09:41mjpieterssetnosy: + mjpieters
messages: + msg356299
2019-04-14 12:35:27obestwaltersetnosy: + obestwalter
messages: + msg340205
2019-02-10 22:15:29cheryl.sabellasetnosy: + cheryl.sabella, lukasz.langa
messages: + msg335174
2018-11-21 01:35:03thorleonsetfiles: + bpo-35278.patch
nosy: + thorleon
messages: + msg330169

2018-11-21 01:20:30python-devsetkeywords: + patch
stage: patch review
pull_requests: + pull_request9875
2018-11-19 14:08:55vstinnersettitle: directory traversal in tempfile prefix -> [security] directory traversal in tempfile prefix
2018-11-19 14:05:50vstinnersetnosy: + vstinner
messages: + msg330100
2018-11-19 12:46:03Yusuke Endohcreate