Lots of badness hides in the string processing.  Horror cases include


stap-client \; ...
stap-client -; ...
stap-client -D 'asdf ; ls /etc' ...
stap-client -e 'script' -D 'asdf ; \; '

So far I haven't found additional small amounts of quoting, and/or
changed eval and such constructs, to make the above not work but keep
ordinary scripts (-e '{..}"..."') work.

It seems clear that a first and necessary step to solving this is to rework the
way the command line information is presented to the server so that it no longer
needs to parse and interpret the command line at all. This is true regardless of
the future of the client and server as standalone scripts/programs or integrated
with stap.

Perhaps something like having each relevant option and its value occupy its own
line in a structured file passed to the server from the client. Perhaps
something in an option:value format. Some examples:

scriptFile:script file name possibly containing spaces.stp
v:2
p:3
unprivileged:

or

e:probe begin { printf ("Hello\n"); exit (); }
unprivileged:

This should be easy for the server to internalize for analysis and it should be
easy for the server to construct and execute the stap command line.

> scriptFile:script file name possibly containing spaces.stp
> v:2
> p:3
> unprivileged:
> e:probe begin { printf ("Hello\n"); exit (); }
> unprivileged:


Right, this is a good way to start thinking about it, but you need to think
even more paranoidly: consider the script containing these lines.

/*
e: nothing 
m: something
*/

roland suggested \0 separated transcriptions of stap-client argv[], which
would be reliable by nature, but /bin/sh is not good at processing such strings.

(In reply to comment #3)

> Right, this is a good way to start thinking about it, but you need to think
> even more paranoidly: consider the script containing these lines.
> 
> /*
> e: nothing 
> m: something
> */

Of course. It was an example, not a spec  :-)
There would still be the required filtering. It would just be a lot easier to
perform.

Here's another approach.  Rewrite stap-client and stap-server as small c++
programs that package the args, almost entirely unparsed, into the .zip file.
(It could be done in perl, but it would add a new rpm dependency, which is
undesirable.)

The new stap-server would invoke stap with a new flag, after which it would
transcribe (carefully) all incoming args from the client:

--client-args -e SCRIPT -v  ... blah

Preceding these would be any extra stap flags that were given to
stap-start-server (-a ARCH etc.), which we can consider trusted.

Then stap itself would look for --client-args, and proceed to parse both
server-side and client-side args together, in good old main.cxx.
The logic for rejecting client-side supplied --unprivileged -D / etc.
would all be here, instead of stap-server/etc.


Dave, until we choose an approach, I'd like you to concentrate on writing 
a good testsuite for this.  It should be in the form of a dejagnu .exp
script that creates junk scripts ("fuzzing") and submits them to
stap & stap-client, to assert equal outputs.  It should arrange to include
special characters like ' ', '\'', '"', '\n', '*', ';', etc. in all the
relevant command line options (-e, -I, ...).

please help test commit series ending 86f99ad, which contains the reworked
server-side parsing/invocation widgetry

Some client-side quoting issues remain, but they do not pose a server-side
security problem.

(In reply to comment #1)
Here is an example which demonstrates the exploit. Running

    stap-client -p1 -B\;ai2

will print the usage help followed by a message similar to

  /usr/local/bin/stap-server: line 340: ai2: command not found

 which indicates that server tried to run the 'ai2' command.

There are additional problem cases that our first patch did not resolve.

The following two patches appear to solve the remaining issues.

http://sourceware.org/git/?p=systemtap.git;a=commit;h=c0d1b5a00
http://sourceware.org/git/?p=systemtap.git;a=commit;h=cc9e5488d