[Patches] Patch to make tempfile return random filenames
Ragnar Kjørstad
ragnark@vestdata.no
Sun, 21 May 2000 21:41:38 +0200
On Sun, May 21, 2000 at 02:25:33PM -0700, Guido van Rossum wrote:
> > 1. the path.exist part does not solve the problem. It causes a racing
> > condition that is not very hard to get around, by having a program
> > creating and deleting the file at maximum speed. It will have a 50%
> > chance of breaking your program.
>
> I'm not sure where you get the 50% chance from. It would seem to
> depend on how often the kernel switches between processes. I'd like
> to see you create a test to decide how big the probability *really*
> is.
Yes, it depends on the context-switches. But if the attacker spawns a
lot of processes it's very likely that at least one of them get a little
cpu-time between the path.exists and open. Given that assumption the
chance is 50%. (The assumption is probably not 100%, so the number is
probably a bit lower - it doesn't change the fact that it is a problem)
> > 2. O_EXCL does not always work. E.g. it does not work over NFS - there
> > are probably other broken implementations too.
>
> But who puts their /tmp directory on NFS?
People without local writable storage. Besides, tempfile uses /usr/tmp
by default. A lot of people have /usr on NFS.
> > 3. Even if mktemp.TemporaryFile had been sufficient, providing mktemp in
> > this dangerous way is not good. Many are likely to use it either not
> > thinking about the problem at all, or assuming it's solved in the
> > module.
>
> I have always assumed this is only really an issue with set-uid
> programs. If I write a simple script, why would it be vulnerable?
> (There's still the PID in the name too.)
You can not assume that the attacker will use python and tempfile.
Finding your pid is easy.
> > 4. The problems you describe can easily be overcome. I removed the
> > counter and the file-exist check because I figgured they were no longer
> > needed. I was wrong. Either a larger number should be used and/or
> > counter and or file-exist check. Personally I would want the random part
> > to bee large enough not have to worry about collisions either by chance,
> > after a fork, or by deliberate attack.
>
> do you know enough about random number generators to know how to
> generate really truly random numbers? randrange(sys.maxint) doesn't
> really help...
No, the seed should be set first. Then I assume it will be random
"enough".
> > Do you want a new patch that adresses theese problems better?
>
> I'm still not convinced that this is really a vulnerability, and that
> you aren't simply translating a known C vulnerability to Python
> without knowing that the same vulnerability exists there.
The only difference I can see is that python beeing slightly slower it's
more likely that the attackers processes get time to create their
symlink between check and open. Are there any other relevant differences
here?
> I'm also
> not convinced that you are enough of a security expert that I should
> trust your recommendations. What are your credentials?
Don't take my word for it, just analyze the problem yourself! If you
want, I can send you a couple of scripts that I used to test this. (with
only one attack-process running, the attack was successful about 1 every
1000 tries - more attack-processes caused more successful attacks.)
--
Ragnar Kjørstad