Robustifying R_CleanTempDir a bit more
On 2/16/23 15:09, Ivan Krylov wrote:
Hello, This is probably a very minor point, but R_CleanTempDir may still have a shell injection in it. I couldn't find a way to shoot the user in the foot in a significant way (by, say, accidentally removing ~), thanks to R disallowing spaces in the path, but if Sys_TempDir somehow acquires a value of "/tmp/';echo;'", R_CleanTempDir() will remove /tmp instead of its aptly-named subdirectory.
Please see 83851 from earlier today which does a bit more of robustification, and if you find any problem in it, please let me know.
While adding the single-quote symbol to the list of special symbols
should suffice (it and the backslash being the only allowed ways to
"un-quote" a single-quoted string), I would like to suggest solving the
problem without the use of quoting:
#include <spawn.h>
char ** argv = { "rm", "-Rf", Sys_TempDir, NULL };
posix_spawnp(NULL, "rm", NULL, NULL, argv, NULL);
Are there Unix-like platforms on which R is intended to work that don't
have posix_spawn()? Circa-2014 versions of both Solaris and OpenBSD
seem to have it. Spawning the process manually by means of [v]fork()
and exec() is probably not worth the maintainer effort required to
perform it correctly.
Yes, this is a good point and we have been thinking about spawn() as well, and we are considering that. Re implementing, I also fear the cost may be too high, thinking about the timeout support in system() I implemented earlier, so essentially a system() replacement for Unix. The details are complicated on Unix as well as on Windows. And re reusing existing implementations, we will have to check they do exactly what we need about signals, terminal, process groups, termination, input and output, etc. It may also be that improving performance of R_unlink() would be easier, as it is rather un-optimized now. So I just wanted to buy time with (possibly temporary) fix in 83851. Thanks Tomas