[MINC-development] patches for tmpnam

[Steve ROBBINS]

On Fri, Mar 07, 2003 at 01:10:38PM -0500, Robert VINCENT wrote:
> On Thu, 6 Mar 2003, Peter NEELIN wrote:
> 
> > BTW, does anyone know to what extent this is a security risk? My guess
> > would be that this is only a risk for either root (or setuid
> > root) applications. Have I missed something?

You're correct that it is a *security* risk only when run by root.
In other instances, the race is a mere inconvenience: the attacker
could make you overwrite any of your files with MINC output.
Here's a lucid discussion on the race and problems with tmpnam

  http://www.linuxselfhelp.com/HOWTO/Secure-Programs-HOWTO/avoid-race.html

 
> So, with thanks to Vicka for pushing the issue, I've made and tested some
> changes I'd like to propose.  In a nutshell, I created a new global minc
> function named "micreate_tempfile()".  I chose this name because I wanted
> it to reflect the fact that the function actually creates the file, but
> closes it before returning.

I'm not clear why it is important to reflect that fact.  Once the file is
closed, the race is on again.


> The function is obsessively #ifdef'd to deal
> with systems that define neither mkstemp() nor tempnam(), but hopefully
> this is just overkill.

Surely checking for tempnam() and tmpnam() is overkill -- MINC has
been used for years without such checking ;-)

It seems to me that you could simplify the logic of the function
by closing the file in the HAVE_MKSTEMP block, and refraining
from creating & closing the file in other cases (since it buys you
nothing).

In addition, you could probably simplify your life if the function
took no arguments and simply returned a pointer to some malloc()'d
memory as the current get_temporary_filename() function does.  This
has two benefits: (a) you wouldn't have to guess at MI_MAX_PATH, and
(b) you can use strlen() before allocating the memory so you don't
have to worry about some fool defining TMPDIR to a really long string
(longer, say, than MI_MAX_PATH).

-S