[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

STINNER Victor added the comment:

Note: Other platform functions still use os.popen(), you might open new issues.

_syscmd_ver:

for cmd in ('ver', 'command /c ver', 'cmd /c ver'):
try:
pipe = os.popen(cmd)
...

def _syscmd_uname(option, default=''):
...
f = os.popen('uname %s 2> %s' % (option, DEV_NULL))

Note: there is also platform.popen() which is deprecated since Python 3.3, we 
might also remove it from Python 3.7 as well (I think that it's now too late 
for Python 3.6).

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

STINNER Victor added the comment:

Oh right, I didn't read the history of the issue. _syscmd_file() doesn't use 
popen() anymore, but subprocess, so the issue can now be closed.

--
resolution:  -> fixed
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Marc-Andre Lemburg]

Marc-Andre Lemburg added the comment:

Shouldn't this ticket be closed ?

platform is using subprocess on both the 2.7 and trunk, so the issue should be 
fixed - and indeed I cannot reproduce it anymore.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

STINNER Victor added the comment:

Christian: would you mind to review my old patch (and rebase it if needed)?

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Christian Heimes]

Christian Heimes added the comment:

I don't see why backwards compatibility with ancient, unsupported Python 
versions should be more important than security. Let's use the subprocess 
module here.

MAL, if you need platform.py for some old projects, please vendor it and use a 
copy.

--
nosy: +christian.heimes
stage: resolved -> needs patch

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Marc-Andre Lemburg]

Marc-Andre Lemburg added the comment:

STINNER Victor wrote:
 
 STINNER Victor added the comment:
 
 The main reason for keeping the compatibility is that the module is
 also being used outside the stdlib for Python versions starting from
 2.4 and later. I don't want to maintain two separate versions.
 
 Which projects use their own copy of platform.py? Where does this file come 
 from?

It's originally from the mxCGIPython project I ran a few years ago
and which now lives on in form of the eGenix PyRun project. Since
the main purpose of platform.py is to identify the underlying Python
runtime and OS platform across multiple Python runtimes and OS platforms,
it's meant to always be used in latest version, even against older Python
versions.

 Using subprocess sounds safer than adding a (theorical) TOCTTOU issue:
 http://en.wikipedia.org/wiki/Time_of_check_to_time_of_use
 
 If you are concerned about compatibility with Python 2.4, you should maintain 
 platform outside the stdlib. The stdlib is regulary updated to the most 
 recent syntax/modules. For example, yield from is now used in the stdlib of 
 Python 3.4.

The Python 2.4 compatibility requirement only applies to Python 2.x,
not Python 3.x.

Since Python 2.4 does ship with subprocess, I guess we could use
subprocess even in Python 2.7, but I'm not sure whether it's a good
idea to change the code in such a major way for a patch level release.

-- 
Marc-Andre Lemburg
eGenix.com

Professional Python Services directly from the Source  (#1, Oct 18 2012)
 Python Projects, Consulting and Support ...   http://www.egenix.com/
 mxODBC.Zope/Plone.Database.Adapter ...   http://zope.egenix.com/
 mxODBC, mxDateTime, mxTextTools ...http://python.egenix.com/

2012-09-27: Released eGenix PyRun 1.1.0 ...   http://egenix.com/go35
2012-09-26: Released mxODBC.Connect 2.0.1 ... http://egenix.com/go34
2012-09-25: Released mxODBC 3.2.1 ... http://egenix.com/go33
2012-10-23: Python Meeting Duesseldorf ...  5 days to go

   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
   Registered at Amtsgericht Duesseldorf: HRB 46611
   http://www.egenix.com/company/contact/

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

STINNER Victor added the comment:

 The main reason for keeping the compatibility is that the module is
 also being used outside the stdlib for Python versions starting from
 2.4 and later. I don't want to maintain two separate versions.

Which projects use their own copy of platform.py? Where does this file come 
from?

Using subprocess sounds safer than adding a (theorical) TOCTTOU issue:
http://en.wikipedia.org/wiki/Time_of_check_to_time_of_use

If you are concerned about compatibility with Python 2.4, you should maintain 
platform outside the stdlib. The stdlib is regulary updated to the most recent 
syntax/modules. For example, yield from is now used in the stdlib of Python 
3.4.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Marc-Andre Lemburg]

Marc-Andre Lemburg added the comment:

Jesús Cea Avión wrote:
 
 Antoine, I agree. I beg your pardon. This patch was suppose to be quite 
 trivial, and test_platform passes just fine on my Linux and Solaris 
 computers. And the four buildbots I was monitoring, the testsuite passed.
 
 The suggestion of Marc-Andre of simply adding an os.access() to check for 
 file existence first is quite sensible and would be enough and compatible in 
 all platforms and python releases.
 
 I am for backing out my patches so far and push a simple os.access() check. 
 What do you think?.

At least for Python 2.7, I think that would be a nice and simple
solution.

 Marc-Andre, I was wondering if you could elaborate a bit why 2.7 platform.py 
 file should be able to run fine in 2.3 :-?

That only applies to the version in Python 2.x. Python 2.3 is perhaps
a bit extreme and I'd be fine with bumping it to 2.4.

The main reason for keeping the compatibility is that the module is
also being used outside the stdlib for Python versions starting from
2.4 and later. I don't want to maintain two separate versions.

-- 
Marc-Andre Lemburg
eGenix.com

Professional Python Services directly from the Source  (#1, Oct 06 2012)
 Python Projects, Consulting and Support ...   http://www.egenix.com/
 mxODBC.Zope/Plone.Database.Adapter ...   http://zope.egenix.com/
 mxODBC, mxDateTime, mxTextTools ...http://python.egenix.com/

2012-09-27: Released eGenix PyRun 1.1.0 ...   http://egenix.com/go35
2012-09-26: Released mxODBC.Connect 2.0.1 ... http://egenix.com/go34
2012-09-25: Released mxODBC 3.2.1 ... http://egenix.com/go33
2012-10-23: Python Meeting Duesseldorf ... 17 days to go

   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
   Registered at Amtsgericht Duesseldorf: HRB 46611
   http://www.egenix.com/company/contact/

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Arfrever Frehtes Taifersar Arahesis]

Changes by Arfrever Frehtes Taifersar Arahesis arfrever@gmail.com:


--
nosy: +Arfrever

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Marc-Andre Lemburg]

Marc-Andre Lemburg added the comment:

Jesús Cea Avión wrote:
 
 Jesús Cea Avión added the comment:
 
 Thanks for the heads-up, Victor.
 
 I have added Marc-Andre Lemburg to the nosy list, so he can know about this 
 issue and can provide feedback (or request a backout for 2.7).
 
 Marc-Andre?.

The comment that Viktor posted still stands for Python 2.7.

You can use subprocess in platform for Python 2.7, but only if
it's available. Otherwise the module must fall back to the
portable popen() that comes with the platform module.

It may be worth adding that selection process to the popen()
function in platform itself.

For Python 3.x, you can use subprocess.

Thanks,
-- 
Marc-Andre Lemburg
eGenix.com

Professional Python Services directly from the Source  (#1, Oct 05 2012)
 Python Projects, Consulting and Support ...   http://www.egenix.com/
 mxODBC.Zope/Plone.Database.Adapter ...   http://zope.egenix.com/
 mxODBC, mxDateTime, mxTextTools ...http://python.egenix.com/

2012-09-27: Released eGenix PyRun 1.1.0 ...   http://egenix.com/go35
2012-09-26: Released mxODBC.Connect 2.0.1 ... http://egenix.com/go34
2012-09-25: Released mxODBC 3.2.1 ... http://egenix.com/go33
2012-10-23: Python Meeting Duesseldorf ... 18 days to go

   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
   Registered at Amtsgericht Duesseldorf: HRB 46611
   http://www.egenix.com/company/contact/

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Marc-Andre Lemburg]

Marc-Andre Lemburg added the comment:

 The implementation of platform.architecture shells out to the file command. 
 It tries to escape quotes by replacing  with \, but that's not sufficient.
 
 $ python3.2 -c 'import platform; platform.architecture(foo\\\; echo Hi 
 there  /tmp/Z; echo \\\)'  cat /tmp/Z
 Hi there
 
 Here's a patch to make it use subprocess instead. I haven't tested it 
 thoroughly building everything from trunk and running tests, but I verified 
 it works by replacing the platform.py in my system Python install.

I think a much better patch would be to test for existence of the
file in question. File names rarely use any of the mentioned quoting
and most certainly not for an executable, so if the check fails, that's
a good indication that something is not right.

Perhaps such a check could be added in addition to the other things
in the patch ?

BTW: It's probably better to discuss such patches on the tracker first,
before applying them to the code base. It becomes difficult discussing
patches that have already been partially applied to the code.

-- 
Marc-Andre Lemburg
eGenix.com

Professional Python Services directly from the Source  (#1, Oct 05 2012)
 Python Projects, Consulting and Support ...   http://www.egenix.com/
 mxODBC.Zope/Plone.Database.Adapter ...   http://zope.egenix.com/
 mxODBC, mxDateTime, mxTextTools ...http://python.egenix.com/

2012-09-27: Released eGenix PyRun 1.1.0 ...   http://egenix.com/go35
2012-09-26: Released mxODBC.Connect 2.0.1 ... http://egenix.com/go34
2012-09-25: Released mxODBC 3.2.1 ... http://egenix.com/go33
2012-10-23: Python Meeting Duesseldorf ... 18 days to go

   eGenix.com Software, Skills and Services GmbH  Pastor-Loeh-Str.48
D-40764 Langenfeld, Germany. CEO Dipl.-Math. Marc-Andre Lemburg
   Registered at Amtsgericht Duesseldorf: HRB 46611
   http://www.egenix.com/company/contact/

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

STINNER Victor added the comment:

1.7 -with open(DEV_NULL) as dev_null:
 1.8 -proc = subprocess.Popen(['file', '-b', '--', target],
 1.9 -stdout=subprocess.PIPE,
stderr=dev_null)
 1.9 +proc = subprocess.Popen(['file', target],
1.10 +stdout=subprocess.PIPE, stderr=subprocess.STDOUT)

Errors should be ignored, not written into stderr. subprocess.DEVNULL
was added to Python 3.3, in older version you have to manually call
open the DEV_NULL file.

(Oh by the way, it should be opened in write mode for stderr, not in
read mode!?)

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

STINNER Victor added the comment:

 You can use subprocess in platform for Python 2.7, but only if
 it's available. Otherwise the module must fall back to the
 portable popen() that comes with the platform module.

Marc-Andre: I still don't understand why you want to run platform.py
of Python 2.7 on older Python version? How does it happen? I expected
Python 2.3 to use its own version of platform.py.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

Changes by STINNER Victor victor.stin...@gmail.com:


--
resolution: fixed - 
status: closed - open

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Antoine Pitrou]

Antoine Pitrou added the comment:

 BTW: It's probably better to discuss such patches on the tracker first,
 before applying them to the code base. It becomes difficult discussing
 patches that have already been partially applied to the code.

Agreed with Marc-André. Jesus, please understand that by committing bogus code, 
you are annoying every other core developer. Be careful and run the test suite 
before pushing any new code. And waiting for reviews doesn't hurt.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Jesús Cea Avión]

Jesús Cea Avión added the comment:

Antoine, I agree. I beg your pardon. This patch was suppose to be quite 
trivial, and test_platform passes just fine on my Linux and Solaris 
computers. And the four buildbots I was monitoring, the testsuite passed.

The suggestion of Marc-Andre of simply adding an os.access() to check for 
file existence first is quite sensible and would be enough and compatible in 
all platforms and python releases.

I am for backing out my patches so far and push a simple os.access() check. 
What do you think?.

Marc-Andre, I was wondering if you could elaborate a bit why 2.7 platform.py 
file should be able to run fine in 2.3 :-?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Jesús Cea Avión]

Jesús Cea Avión added the comment:

This is security risk, but I can't think about any platform.architecture() 
use that would use a untrusted parameter. Nevertheless os.popen() has been 
deprecated for ages. I take care of this.

--
assignee:  - jcea
nosy: +jcea
versions:  -Python 2.6, Python 3.1, Python 3.5

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Roundup Robot]

Roundup Robot added the comment:

New changeset c73b90b6dadd by Jesus Cea in branch '2.7':
Closes #16112: platform.architecture does not correctly escape argument to 
/usr/bin/file
http://hg.python.org/cpython/rev/c73b90b6dadd

New changeset 6c830b657900 by Jesus Cea in branch '3.2':
Closes #16112: platform.architecture does not correctly escape argument to 
/usr/bin/file
http://hg.python.org/cpython/rev/6c830b657900

New changeset 3112bf7e0ecb by Jesus Cea in branch '3.3':
MERGE: Closes #16112: platform.architecture does not correctly escape argument 
to /usr/bin/file
http://hg.python.org/cpython/rev/3112bf7e0ecb

New changeset cd026866b333 by Jesus Cea in branch 'default':
MERGE: Closes #16112: platform.architecture does not correctly escape argument 
to /usr/bin/file
http://hg.python.org/cpython/rev/cd026866b333

--
nosy: +python-dev
resolution:  - fixed
stage:  - committed/rejected
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Jesús Cea Avión]

Jesús Cea Avión added the comment:

I have a bootstrap problem with python 2.7. Backout that patch, for now.

--
resolution: fixed - 
status: closed - open

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Roundup Robot]

Roundup Robot added the comment:

New changeset d6d908dc11f2 by Jesus Cea in branch '2.7':
Closes #16112: platform.architecture does not correctly escape argument to 
/usr/bin/file. Solve a 2.7 bootstrap issue
http://hg.python.org/cpython/rev/d6d908dc11f2

--
resolution:  - fixed
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Jesús Cea Avión]

Jesús Cea Avión added the comment:

Thanks for bringing this up and for your patch.

--
resolution: fixed - 
status: closed - open

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

STINNER Victor added the comment:

I did a similar commit two years ago, but I reverted it because I failed to 
find a solution for the bootstrap issue.

changeset:   60673:7c90ac194e40
branch:  legacy-trunk
parent:  60664:d7d5c76545fb
user:Victor Stinner victor.stin...@haypocalc.com
date:Sun Apr 18 09:07:49 2010 +
files:   Lib/platform.py
description:
platform: use subprocess.Popen() instead of os.popen() in _syscmd_file()

 * Popen() avoids ugly shell escape: target.replace('', '\\')
 * Use proc.communicate() instead of f.stdout.read()
 * Get output from stdout by splitting with :  instead of splitting by spaces
   to support filename with spaces

--
nosy: +haypo

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Jesús Cea Avión]

Changes by Jesús Cea Avión j...@jcea.es:


--
resolution:  - fixed
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

STINNER Victor added the comment:

See also issue #9560 and the following email from Marc-Andre Lemburg.
http://mail.python.org/pipermail/python-checkins/2010-April/092099.html

Viktor, before making such changes to platform.py, please coordinate
with me, as I am the maintainer of that module.

The subprocess change is not OK, since platform.py has to stay
compatible with Python 2.3 which doesn't have subprocess. Please either
revert the change or make it fallback to os.popen().

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Jesús Cea Avión]

Jesús Cea Avión added the comment:

Thanks for the heads-up, Victor.

I have added Marc-Andre Lemburg to the nosy list, so he can know about this 
issue and can provide feedback (or request a backout for 2.7).

Marc-Andre?.

--
nosy: +lemburg

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Antoine Pitrou]

Antoine Pitrou added the comment:

This change broke test_platform for me:

==
ERROR: test_architecture (test.test_platform.PlatformTest)
--
Traceback (most recent call last):
  File /home/antoine/cpython/32/Lib/test/test_platform.py, line 11, in 
test_architecture
res = platform.architecture()
  File /home/antoine/cpython/32/Lib/platform.py, line 1072, in architecture
if 'executable' not in fileout:
TypeError: Type str doesn't support the buffer API

--
nosy: +pitrou
status: closed - open

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

STINNER Victor added the comment:

Reopen: test_platform is failing. Attached patch should fix the issue. It fixes 
a theoric deadlock issue in _syscmd_file: use proc.communicate() instead of 
proc.stdout.read().

--
resolution: fixed - 
Added file: http://bugs.python.org/file27427/platform.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[David Benjamin]

David Benjamin added the comment:

Well, the theoretical deadlock's just if stdin is also a pipe, right? I think 
there isn't be a difference between communicate and stdout.read if only stdout 
is a pipe. Though it might be worth passing DEVNULL to stdin instead of 
inheriting, just to be tidy.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

STINNER Victor added the comment:

 Well, the theoretical deadlock's just if stdin is also a pipe, right?

If I remember correctly, the parent process (Python) and the child
process (file) can be blocked if the child is blocked in a blocking
write into a pipe (ex: stderr), whereas the pipe buffer is full (the
size of the buffer is usually 4096 bytes) and the parent is reading
all content of another pipe (ex: stdout). So it occurs if the process
uses more than 1 pipe for standard input/output streams. In the case
of platform, it looks like the issue cannot occur.

I always prefer communicate() because I consider it safer :-)

 Though it might be worth passing DEVNULL to stdin instead of inheriting, just 
 to be tidy.

Yeah, maybe. As you want.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Antoine Pitrou]

Antoine Pitrou added the comment:

Victor: your patch doesn't apply on 3.2, but it works for me on 3.3.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Antoine Pitrou]

Antoine Pitrou added the comment:

Victor is right, communicate() is usually the right way to do it. No need to 
use unsafe idioms.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[STINNER Victor]

STINNER Victor added the comment:

 Victor: your patch doesn't apply on 3.2, but it works for me on 3.3.

I wrote it for Python 3.4. I'm too lazy for adapt it to other branches.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[David Benjamin]

David Benjamin added the comment:

Yes, communicate is needed if you have multiple pipes and need to be careful 
about both ends doing a blocking reads/writes on different ones. There's only 
one pipe here. Eh, whatever. If you guys really want to use communicate, I 
don't really care.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Roundup Robot]

Roundup Robot added the comment:

New changeset b94a9ff13199 by Jesus Cea in branch '2.7':
#16112: platform.architecture does not correctly escape argument to 
/usr/bin/file. Use 'communicate()'
http://hg.python.org/cpython/rev/b94a9ff13199

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Roundup Robot]

Roundup Robot added the comment:

New changeset 04f39958aea9 by Jesus Cea in branch '3.2':
#16112: platform.architecture does not correctly escape argument to 
/usr/bin/file. Use 'communicate()' and decode the bytes
http://hg.python.org/cpython/rev/04f39958aea9

New changeset 19d37c8d1882 by Jesus Cea in branch '2.7':
#16112: platform.architecture does not correctly escape argument to 
/usr/bin/file. Fix original patch
http://hg.python.org/cpython/rev/19d37c8d1882

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Roundup Robot]

Roundup Robot added the comment:

New changeset 9838ae397a19 by Jesus Cea in branch '3.2':
#16112: platform.architecture does not correctly escape argument to 
/usr/bin/file. Fix original patch
http://hg.python.org/cpython/rev/9838ae397a19

New changeset 64a0caf49429 by Jesus Cea in branch '3.3':
MERGE: #16112: platform.architecture does not correctly escape argument to 
/usr/bin/file. Fix original patch
http://hg.python.org/cpython/rev/64a0caf49429

New changeset b9ac3c44a4eb by Jesus Cea in branch 'default':
MERGE: #16112: platform.architecture does not correctly escape argument to 
/usr/bin/file. Fix original patch
http://hg.python.org/cpython/rev/b9ac3c44a4eb

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Jesús Cea Avión]

Jesús Cea Avión added the comment:

David, after so many patches, can you confirm that we have solved the original 
issue? :)

--
resolution:  - fixed
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[Jesús Cea Avión]

Jesús Cea Avión added the comment:

Why I was not seeing test_platform.py failing in the buildbots? :-?

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue16112] platform.architecture does not correctly escape argument to /usr/bin/file

[David Benjamin]

New submission from David Benjamin:

The implementation of platform.architecture shells out to the file command. It 
tries to escape quotes by replacing  with \, but that's not sufficient.

$ python3.2 -c 'import platform; platform.architecture(foo\\\; echo Hi there 
 /tmp/Z; echo \\\)'  cat /tmp/Z
Hi there

Here's a patch to make it use subprocess instead. I haven't tested it 
thoroughly building everything from trunk and running tests, but I verified it 
works by replacing the platform.py in my system Python install.

--
components: Library (Lib)
files: fix-platform-architecture.patch
keywords: patch
messages: 171825
nosy: David.Benjamin
priority: normal
severity: normal
status: open
title: platform.architecture does not correctly escape argument to /usr/bin/file
type: security
versions: Python 2.6, Python 2.7, Python 3.1, Python 3.2, Python 3.3, Python 
3.4, Python 3.5
Added file: http://bugs.python.org/file27391/fix-platform-architecture.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue16112
___
___
Python-bugs-list mailing list
Unsubscribe: 
http://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com