4 ms·
> Oops, looks like its tempfile lib is written in library code in the style of mktemp(3), instead of calling the OS’s mkstemp(3) or equivalent. So it just finds
by sedatk 2y ago
> Oops, looks like its tempfile lib is written in library code in the style of mktemp(3), instead of calling the OS’s mkstemp(3) or equivalent. So it just finds a filename that doesn’t exist and then opens it separately, letting an attacker create a new file which they can read at that location between those two steps. Uh, have fun with your temporary files leading to exploitable race conditions guys.
Good catch. The API should be opening the tempfile atomically (the best option), or at least should use a CSPRNG for its random suffix. It doesn’t, currently.
- bhaney 2y agoI don't think that complaint is even true. A quick glance at the Crystal stdlib source[1] shows that the mktmp implementation uses the O_EXCL mode, which will cause the open call to error out if the file already exists, and the implementation here will handle that and try again with a different filename. I think the worst thing an attacker could do here if they could somehow predict the generated names before the file is created is DoS the mktmp call by racing ahead of it 100 times until it gives up. [1] https://github.com/crystal-lang/crystal/blob/release/1.13/src/crystal/system/file.cr#L53 https://github.com/crystal-lang/crystal/blob/release/1.13/sr...
- sedatk 2y agoAlso, good catch! :)