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
>