elixir-plug / plug

Compose web applications with functions
https://hex.pm/packages/plug
Other
2.84k stars 582 forks source link

Plug.Upload's random-tempdir generation is not very random #1179

Closed icefoxen closed 10 months ago

icefoxen commented 10 months ago

I had an issue where I was running two different instances of the same server as different users (one for prod, one for dev/test) and only whichever one of them started first could upload temp files. Upon inspection it always tries to write to the temp dir /tmp/plug-1697, no matter what, so we have two programs both trying to write to the same dir. They're run as different users, so whichever is started first starts the dir and owns it, and the other user can't write to it so the upload fails.

Looks like the temp dir name is generated by this code in plug/lib/plug/upload.ex:

  defp generate_tmp_dir() do
    tmp_roots = :persistent_term.get(__MODULE__)
    {mega, _, _} = :os.timestamp()
    subdir = "/plug-" <> i(mega)
   ...

On my Linux system, tmp_roots is ["/tmp"], which is fine, and :os.timestamp() is (currently) {1697, 828389, 981738}, which is less fine. The first number is megaseconds, so it will only tick over every 11.5 days.

Annoyingly, neither Erlang nor Elixir appear to provide a binding to the mkdtemp libc function, which is there for exactly this purpose. Making temp files/dirs safely has all sorts of horrible edge cases involved, at least on Unix. It looks like OpenBSD libc implements mkdtemp as just trying random numbers a bunch of times, and musl libc does the same with a worse RNG, so it's pretty cursed but I guess that's life. So getting the suffix from :rand.rand_uniform/1 or :crypto.rand_uniform/2 is probably as good as one can do.

icefoxen commented 10 months ago

oh heck, thank you!