[Python-Dev] Weird error handling in os._execvpe

[Guido van Rossum]

> > > Can anyone explain why it is done this way?
> > 
> > Because not all systems report the same error for this error condition
> > (attempting to execute a file that doesn't exist).
> 
> That's unfortunate.  The existing code is buggy on at least three
> grounds:
> 
> First and most important, it's currently trivial to cause any program
> that uses os.execvp[e] to invoke a program of the attacker's choice,
> rather than the intended one, on any platform that supports symbolic
> links and has predictable PIDs.  My tempfile rewrite will make this
> much harder, but still not impossible.

That's important.

> Second, the BeOS code will silently delete the file '/_#.# ## #.#' if
> it exists, which is unlikely, but not impossible.  A user who had
> created such a file would certainly be surprised to discover it gone
> after running an apparently-innocuous Python program.

I really don't care about that. :-)

> Third, if an error other than the expected one comes back, the loop
> clobbers the saved exception info and keeps going.  Consider the
> situation where PATH=/bin:/usr/bin, /bin/foobar exists but is not
> executable by the invoking user, and /usr/bin/foobar does not exist.
> The exception thrown will be 'No such file or directory', not the
> expected 'Permission denied'.

Hm, you're right.  The code (which I believe I wrote, except for the
BeOS bit) was attempting to get the opposite effect, but seems to be
broken. :-(

> Also, I'm not certain what will happen if two threads go through the
> if not _notfound: block at the same time, but it could be bad,
> depending on how much implicit locking there is in the interpreter.
> 
> I see three possible fixes.  In order of personal preference:
> 
> 1. Make os.execvp[e] just call the C library's execvp[e]; it has to
>    get this stuff right anyway.  We are already counting on it for
>    execv - I would be surprised to find a system that had execv and
>    not execvp, as long as PATH was a meaningful concept (it isn't, for
>    instance, on classic MacOS).

Probably agreed for execvpe().  All the non-env versions must call the
env version because not all platforms have putenv, and there changes
to os.environ don't get reflected in the process's environment.

> 2. Enumerate all the platform-specific errno values for this failure
>    mode, and check them all.  On Unix, ENOENT and arguably ENOTDIR.  I
>    don't know about others.
> 
> 3. If we must do the temporary file thing, create a temporary
>    _directory_; we control the contents of that directory, so we can
>    be sure that the file name we choose does not exist.  Cleanup is
>    messier than the other two possibilities.

I like to agree with this, but I don't recall exactly why we ended up
in this situation in the first place.  It's possible that it's an
unnecessary sacrifice of a dead chicken, but it's also possible that
there are platforms where this addressed a real need.  I'd like to
think that it was because I didn't want to add more cruft to
posixmodule.c (I've long given up on that :-).

Can you post a patch to SF?  Then we can ask for volunteers to test it
on various platforms.

--Guido van Rossum (home page: http://www.python.org/~guido/)