Use system2 to avoid shell injection

Line 121 in 7b9c1ea

cmd <- sprintf('%s install %s', findBower(), pkg)

IINM this could be subject to shell injection if pkg can be passed as an arbitrary input.

system2() should have roughly the same level of code complexity & avoids this issue

Could you elaborate on the threat model here? If someone's in a position to call this function, I'd expect that they would already be able to run arbitrary R code, which means they could call system() and do what ever they want.

What's preventing someone from leaving an API into this function from e.g. a shiny app?

is there a reason to prefer system() here? it seems the output is not captured anyway so the behavior AIUI should be the same

htmlwidgets/R/scaffold.R

Line 121 in 7b9c1ea

cmd <- sprintf('%s install %s', findBower(), pkg)

IINM this could be subject to shell injection if pkg can be passed as an arbitrary input.

system2() should have roughly the same level of code complexity & avoids this issue

Are you sure system2() is any safer in this aspect? I would say it may be more convenient, but it seems to me its implementation uses shQuote() on the command, pastes all args together and calls an internal system() call.

https://github.com/wch/r-source/blob/0987da15dd567ad07f91745238588c7873844d4c/src/library/base/R/unix/system.unix.R#L56

Reading that code makes me believe that system2() also creates a shell to run the given command, and therefore it is vulnerable to shell parsing as well.

I could try to write a system2() call to prove this once I'm not on my phone if you need it, but the source linked above seems quite straightforward to follow.

Please correct me if I am wrong but it seems to me that injecting code in system2() is perfectly doable through the 'args'

Indeed that's right. Should we just run shQuote() on pkg ourselves, in that case?

Indeed that's right. Should we just run shQuote() on pkg ourselves, in that case?

If I had to run an external process without a shell I would use processx::run() which does not use a shell and by definition avoids shell injection issues.