Temporary files directory name is predictable

[exeter-matthew-wakeling]

The temporary files directory name is very predictable, while appearing to be random. Most of the time, the directory name is tmp.fa37JncCHr. This could potentially be a security issue. It would be better if the temporary file directory had a more random name that cannot be predicted by a potential attacker.

As an example of an attack:

1. User A starts spring.
2. User A's spring checks for the existence of tmp.fa37JncCHr.
3. User B creates tmp.fa37JncCHr as a symlink pointing to another directory that they do not have write access to.
4. User A's spring tries to create the directory tmp.fa37JncCHr but fails.
5. User A's spring starts writing files into the directory, potentially damaging existing files in the directory user B linked it to.
   This allows user B to cause writes to appear in a directory they do not have write access to.

[shubhamchandak94]

Hi, thanks for the investigation. In a recent commit, I added a check where Spring will keep trying new paths if it fails to create due to path already existing. So I'm not sure the transition from step 4 to 5 will happen as you describe.

[exeter-matthew-wakeling]

That's likely true, and that's a good check. However, there are reasons why unpredictable temporary file names are recommended that are beyond my current ability to think about right now, and it appears that it should be fairly easy to make them unpredictable and no longer have to worry about whether you have done enough to protect yourself.

The other thing I noticed is that the temporary directory appears to be given permissions for group to write, which opens up the exact same attack, because the attacker could create symlinks in the temporary directory that spring then clobbers for the attacker, and those file names are very predictable. It'd be best if the temporary directory had a mode of 700, denying read/write permissions to everyone but the person running spring.

[exeter-matthew-wakeling]

Oh, it's not enough to check for symlinks either, because hard links will do the same thing too.

[shubhamchandak94]

Thanks, let me investigate this.

[olawa]

I had the same issue when decompressing multiple files at the same time.

Using the base name of the spring file soved it for me:
std::string random_str = "tmp." + spring::random_string(10) + outfile_vec[0].substr(outfile_vec[0].find_last_of("/\") + 1)