Python CGI security

[Thomas Wouters]

On Mon, 03 Jul 2000 11:22:50 +0200, Michael Ströder <michael at stroeder.com>
wrote:
>Thomas Wouters wrote:
>> On Mon, 03 Jul 2000 08:47:47 +0200, Michael Ströder <michael at stroeder.com>
>> wrote:
>> >"E. Jordan Bojar" wrote:
>> 
>> >> [..] buffer overflow [..] 
>> 
>> >I'm not a C programmer at all and therefore I just naively used a
>> >tool named ITS4: <URL: http://www.rstcorp.com/its4/> to find parts
>> >in the Python interpreter which might have buffer overrun issues. I
>> >would really like to have some discussion about the ITS4-output.
>> 
>> If you do want to have that discussion, feel free to send me that report, 
>> or post it here

>Ok, here are some examples from Python1.6a2 sources (see below). I'm
>not a C programmer therefore such tools are my only possibility to
>raise some discussion by pointing to code. Off course the tool
>outputs only hints. Not every code line listed is a real security
>hole.

>I understand that the Python application itself is responsible for
>dealing with some issues (e.g. race conditions with files) since the
>Python libs only wrap some system libs. And I understand that some
>of the listed functions as being dangerous are used in a safe way
>with appropriate parameters.

Indeed. And a couple of the 'security issues', for instance those with
popen() and system(), and the race conditions in stat/access etc, Python
itself does not care to solve, but instead delegates to the scripts it is
executing ;)

Python will not run set-uid out of the box. You have to supply your own
setuid-wrapper if you want it to, in which case you should also take care
with those functions. None the less, the seperation of environment variables
and system() and popen() makes it slightly less insecure than C.

Also, this suite you used seems to be targeted towards data-handling
engines, like webservers, in a hostile environment. This is entirely unlike
Python's usual environment. If you plan on programming in such environments,
you should take care with your programs. Python will not do the Right Thing
each time, but it also does not pretend it does. The habit of throwing
exceptions on errors instead of returning an undefined value and continueing
on helps prevent a lot of tight spots, though.

But I'll do a quick run through the 'hotspots':

>----------------------------------------------------------------------
>Modules/posixmodule.c:248:(Urgent) popen
>Easy to run arbitrary commands through env vars.
>Use fork + execve + pipes instead.
>----------------

See above. popen() is supplied to Python scripts, but python itself does not
use it. Using popen() yourself should ofcourse be done carefully :P

>----------------
>Modules/posixmodule.c:1151:(Urgent) system
>Easy to run arbitrary commands through env vars. Also, potential
>TOCTOU
>problems.
>Use fork + execve instead.
>----------------

See popen()

>----------------
>Modules/posixmodule.c:605:(Very Risky) access
>Potential race condition on: path
>Points of concern are:
>Modules/posixmodule.c:605: access
>Modules/posixmodule.c:701: chmod
>Modules/posixmodule.c:1063: mkdir
>Modules/posixmodule.c:1065: mkdir
>Modules/posixmodule.c:1254: utime
>Modules/posixmodule.c:2235: readlink
>Modules/posixmodule.c:3263: statvfs
>Manipulate file descriptors, not symbolic names, when possible.
>----------------

See popen(). If you stat for a filename, you should be aware that it can go
away from under you. 

>----------------
>Modules/main.c:117:(Very Risky) getenv
>Often seen in conjunction with buffer overflows, etc.
>Remember that env vars can contain arbitrary malicious input.  Test
>accordingly
>before use.
>----------------

Good warning for every programmer in hostile environments, but not
applicable to python itself. Usually Python scripts are not setuid, so they
can trust their environment (they run under the same permission as who
created that environment, after all.) If you use a setuid wrapper, you
should pass a new, 'safed' environment yourself.

>----------------
>Modules/posixmodule.c:1863:(Very Risky) getlogin
>It is very easy to fool.
>Don't trust its output.
>----------------

See getenv().

>----------------
>Modules/parsermodule.c:1032:(Very Risky) sprintf
>This function is high risk for buffer overflows
>Use snprintf if available, or precision specifiers, if available.
>----------------

Used as safely as necessary. The parser module is not something quickly used
in high security environments in any case.

>----------------
>Modules/getpath.c:186:(Very Risky) stat
>Potential race condition on: filename
>Points of concern are:
>Modules/getpath.c:186: stat
>Modules/getpath.c:216: stat
>Modules/getpath.c:231: stat
>Modules/main.c:194: fopen
>Python/bltinmodule.c:883: fopen
>Python/pythonrun.c:583: fopen
>Python/traceback.c:157: fopen
>Manipulate file descriptors, not symbolic names, when possible.
>----------------

See access

>----------------
>Modules/posixmodule.c:385:(Very Risky) strcat
>This function is high risk for buffer overflows
>Use strncat instead.
>----------------

String lenghts are passed in and recorded seperately, and though I haven't
backtracked it the whole way, they seem to be properly kept up to date. This
is an OS/2 only branch, though.

>----------------
>Modules/posixmodule.c:827:(Very Risky) strcpy
>This function is high risk for buffer overflows
>Use strncpy instead.

String lengths are explicitly checked.