[Rd] Robustifying R_CleanTempDir a bit more

[Tomas Kalibera]

On 2/16/23 15:43, Tomas Kalibera wrote:
> 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've added the single quote now. Thanks for spotting that. This is a 
temporary fix which may be later replaced by spawn or something else.

Best
Tomas

>> 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
>
>>