Editing text with an external editor in Python

[Steven D'Aprano]

Roy Smith wrote:

> Hmmm.  Didn't we just have a thread about passing external data to
> shells?
> 
> $ mkdir '/tmp/;rm -rf;'
> $ TMPDIR='/tmp/;rm -rf;' python
> Python 2.7.3 (default, Sep 26 2013, 20:03:06)
> [GCC 4.6.3] on linux2
> Type "help", "copyright", "credits" or "license" for more information.
>>>> import tempfile
>>>> f = tempfile.NamedTemporaryFile()
>>>> f.name
> '/tmp/;rm -rf;/tmpW8HFTr'

Seems like a lot of trouble to go to to erase your own system. Couldn't you
just run rm -rf / on your own system prior to launching Python?

But seriously, I'm not sure what attack vector you think you have found.
By definition, this is calling out to an external application, which might
do *anything*. It needs to be used in a trusted environment, like any other
tool which calls out to external applications.

[steve at ando ~]$ mkdir foo
[steve at ando ~]$ cd foo
[steve at ando foo]$ git init
Initialized empty Git repository in /home/steve/foo/.git/
[steve at ando foo]$ echo Some content > stuff.txt
[steve at ando foo]$ git add stuff.txt
[steve at ando foo]$ GIT_EDITOR="echo 'you got pwned' #" git commit
you got pwned
Aborting commit due to empty commit message.


I'm not really seeing how this is a security vulnerability. If somebody can
break into my system and set a hostile GIT_EDITOR, or TMPDIR, environment
variables, I've already lost.

As written, the edit() function takes two arguments: the name of an
application to call, and optionally initial contents to be edited.
Obviously the name of the application has to be trusted: either hard code
it, or get it from a trusted source like the VISUAL or EDITOR environment
variables. (It's *your* environment, you can set them to whatever editor
you want. If you want to set them to something hostile, that's your
prerogative.)

If an attacker has already compromised my editor, I've lost.

If I naively run arbitrary code provided by *untrusted* sources (say, I get
the editor from anonymous users over the internet), I've lost. I didn't
think I needed to explicitly say that.

On the other hand, the initial content need not be trusted, since it's just
text. The worst somebody could do is hurt my feelings. (Well, they could in
principle buffer-overflow the editor, or Python, but if you can't trust
Python and your editor to be resistant to that, you shouldn't use them.) If
the initial content could "leak" out and do bad things, that would be a
vulnerability I care about. Having the user destroy their own system by
deliberately misusing the function is not my problem:

# Don't do this. It would be bad.
edit("rm -rf / #")


Have I missed something? I really don't think this is a vulnerability, and I
don't see how using the subprocess module would make it safer.


-- 
Steven