classification
Title: Consistently handle path separator in Py_GetPath on Windows
Type: security Stage: patch review
Components: Interpreter Core, Windows Versions: Python 3.3, Python 3.2, Python 3.1, Python 2.7, Python 2.6
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Nam.Nguyen, barry, benjamin.peterson, eric.araujo, flox, gvanrossum, haypo, loewis, mhammond, terry.reedy
Priority: normal Keywords: needs review, patch

Created on 2011-09-16 00:08 by Nam.Nguyen, last changed 2012-01-07 15:40 by eric.araujo.

Files
File name Uploaded Description Edit
getpath.consistent.delim.patch Nam.Nguyen, 2011-09-16 00:08 make path delimiter handling in getpathp.c consistent
Messages (6)
msg144111 - (view) Author: Nam Nguyen (Nam.Nguyen) Date: 2011-09-16 00:08
The module search path is constructed from PYTHONPATH env-var, then zip path, then HKCU PythonPath, then HKLM PythonPath, then PYTHONPATH define (in pyconfig.h), and finally argv[0]. If PYTHONHOME is available, the PYTHONPATH define is expanded. These paths are separated by semicolon.

Without PYTHONHOME, PYTHONPATH define is appended to module_search_path as-is, and a semicolon comes **after** that. With PYTHONHOME, PYTHONPATH define is expanded, and there is no semicolon after it. Then, finally, when argv[0] is added to module_search_path, a semicolon is **prepended** before it.

This inconsistency in handling path delimiter leads to a case where two semicolons are next to each other (;;), which is translated to the current directory. It happens when PYTHONHOME is not found. The current directory is put in front of the application directory (argv[0]) causing a security issue whereby external modules might be imported inadvertently.

This patch makes semicolon handling consistent. A semicolon is appended at the end of every path component, except argv[0].
msg144161 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-09-16 21:00
Barry, Benjamin, do you agree that this is a security issue as far as future 2.6 and 3.1 security fix releases are concerned?
msg144304 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2011-09-19 17:45
I'm not Barry or Benjamin, but having followed the thread on psrt@python.org, this certainly looks like a security issue to me. As a second pair of eyes, I recommend MvL, who builds our Windows installers.
msg144305 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2011-09-19 17:50
Approved for 3.1 as far as I'm concerned.
msg144311 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2011-09-19 21:36
Brian, you marked this 'patch review', bypassing 'test needed'. Should this have any visible effect at the Python level that can be tested?
msg145989 - (view) Author: Mark Hammond (mhammond) * (Python committer) Date: 2011-10-19 23:01
The first chunk of that patch is for when pythonhome==NULL.  There is also a similar block just under it when MS_WINDOWS is not defined.  While I don't know in which cases this will be built without that define, it looks as though the *buf++ = DELIM; should also be added to that block?

Also, there is an existing conditional:
     if (argv0_path) {

which can never be false (as argv0_path is a char array).  It could be changed to if (argv0_path[0]) but ISTM that it could also be removed completely - ie, the 2 lines left in that block should have no effect in the case where the buffer is empty.

I haven't actually tested it though, but apart from the first comment above, it *looks* like it does the right thing :)
History
Date User Action Args
2012-01-07 15:40:49eric.araujosetnosy: + eric.araujo
2011-10-19 23:01:13mhammondsetnosy: + mhammond
messages: + msg145989
2011-10-19 17:37:03floxsetnosy: + flox
2011-10-19 16:23:49hayposetnosy: + haypo
2011-09-19 21:36:54terry.reedysetmessages: + msg144311
2011-09-19 17:50:36benjamin.petersonsetmessages: + msg144305
2011-09-19 17:45:53gvanrossumsetnosy: + gvanrossum, loewis
messages: + msg144304
2011-09-16 21:00:01terry.reedysetnosy: + barry, terry.reedy, benjamin.peterson
messages: + msg144161
2011-09-16 02:38:03brian.curtinsetkeywords: + needs review
stage: patch review
type: security
versions: - Python 3.4
2011-09-16 00:08:15Nam.Nguyencreate