KProcess, KRun, KShell, KMacroExpander, their abuse, windows and security holes
js at iidea.pl
Fri Mar 28 21:07:39 GMT 2008
Oswald Buddenhagen said the following, On 2008-03-16 22:40:
> first of all, a rant: i did a grep on KShell::quoteArg ... the number of
> hits is amazing. and 90+% indicate unnecessary use of a shell.
> the top three reasons *not* to go through a shell:
> - it is not portable
> - it is slow
> - you risk security holes (if you get the argument quoting wrong) -
> remember that delayed release some years ago?
> so if your code contains any of these:
> - system()
> - popen()
> - KProcess::setShellCommand() or K3Process::setUseShell(true)
> you are likely to be doing something wrong. please review and fix any
> code that you maintain.
> on a related note, KMacroExpander::expandMacrosShellQuote() and
> KShell::splitArgs() are still under-used, leading to lots of duplicated
> (and usually not particularly robust) code. but see below.
> now to the broader topic ... starting subprocesses and supplying
> arguments to them. for now only unix ...
> - the basic and usually only correct way to start a process is by using
> KProcess (without resorting to setShellCommand). you can run processes
> synchronously, asynchronously or completely detached, supply a working
> dir, make redirections, chain processes and deal with their i/o.
> - if you have the user supply file names, you might want to run them
> through KShell::tildeExpand() prior to passing them to the process.
> but KFileDialog already handles this internally, so usually there is
> no need to bother with it (and expanding strings that are not meant to
> be would be potentially harmful, anyway).
> - then there is KShell::splitArgs() (used without AbortOnMeta). it
> performs word-splitting according to somewhat simplified bash rules.
> the result is usually fed into a KProcess.
> meanwhile it is even somewhat popular.
> but one has to wonder what it is actually used for - why would anyone
> supply something that resembles a shell command, but is none?
> - then exists the possibility to run a complete command through a shell
> via KProcess::setShellCommand().
> - if you use KShell::quoteArg() or KShell::joinArgs() to construct a
> command line, you should reconsider - see above.
> - a generally justified use of the shell are user-supplied command
> lines. often such a command line is filtered through
> KMacroExpander::expandMacrosShellQuote() prior to passing it to
> KProcess to replace any expandos in the command.
> - a somewhat fancy way to execute a shell command is
> - a fairly efficient way to execute a shell command is using
> KShell::splitArgs() with the AbortOnMeta flag - if it can be parsed
> according to the simplified bash rules, it can be run directly,
> otherwise it needs to be run through the shell.
> KRun does that internally.
> now comes into play this bloody braindead waste of time that is called
> windows. it pretty much screws us over with everything that actually
> requires using a shell. the gory details are explained in
> as a consequence i suggest the following course of action:
> - i would build the KShell::splitArgs() with AbortOnMeta magic from KRun
> directly into KProcess::setShellCommand(). on unix, a flag to bypass
> this automagic would exist. on windows, it would simply refuse to
> run anything too complex - users can be expected to use batch files for
> complex constructs anyway.
> to KRun::runCommand() the same would apply.
> - KShell::splitArgs() would disappear from the public api on windows. on
> unix, its use should be limited. it would be mostly obsoleted by the
> previous point anyway.
> - any code that uses KShell::quoteArg() or KShell::joinArgs() is likely
> to be either bogus or inherently platform-dependend. i think it
> should be made unix-only again.
> - KMacroExpander::expandMacrosShellQuote() would be safe only as far as
> the improved KProcess::setShellCommand() goes.
> anything i did not consider?
I can only say that the fact that bool
KMacroExpanderBase::expandMacrosShellQuote( QString &str,
int &pos ) just fails on Windows makes KRun::processDesktopExec() failing too,
what in turn breaks opening in my KMail :).
On windows it's true that 99.(9)% of use cases for symbol expanding is
"<command> %f" call, what's indeed the pattern used so often in the windows
regards / pozdrawiam, Jaroslaw Staniek
Sponsored by OpenOffice Polska (http://www.openoffice.com.pl/en) to work on
Kexi & KOffice (http://www.kexi.pl/en, http://www.koffice.org/kexi)
KDE Libraries for MS Windows (http://windows.kde.org)
More information about the kde-core-devel