Bug#276419: [Pkg-shadow-devel] Bug#276419: su appends the positional args to the command line

[Helmut Waitzmann (Debian Bug Tracking System)]

Christian Perrier <bubulle at debian.org> writes:

>We will make this transition

[...]

>What I fear a little is the discussion that is likely to follow in
>-develwith people nitpicking the arguments we have for doing the
>change.
>
>*Here* I expect you, shadow maintainers AND Helmut, to help me in
>explaining with the Right Arguments, why we have to change this. 

Quoting the manual page:

   SU(1)

   NAME
	  su - Change user ID or become super-user

   SYNOPSIS
	  su [OPTS] [-] [username [ARGS]]


What are the pros and cons in doing the change to new su?

To be able to use arbitrary shell invocation options, e.g. start a login
shell in debug mode:

$ su -- - username -x

Start a login shell with additional positional parameters, which can be
examined in "$HOME"/.profile:

$ su -- - username -s XAUTHORITY="$XAUTHORITY"

These are impossible with old su.  So it turns out to be a PRO.



With old su,

$ su -- root cat /etc/shadow

works as expected.  With new su, it won't work.  To make it work with new
su, one has to concatenate the commandline by oneself and pass it as a
parameter to the '-c' option:

$ su -c cat\ /etc/shadow -- root

This is related to problems with proper commandline quoting that have
been reported, as can be seen for example in the posts to
<317264 at bugs.debian.org> with message-ids
<874qb6mgdj.dancerj at netfort.gr.jp>,
<lflr7e5ny9h.fsf at helmutwaitzmann.news.arcor.de>,
<871x65gktx.dancerj at netfort.gr.jp>, and
<lflk6jvcjj2.fsf at helmutwaitzmann.news.arcor.de>.

Quoting from <lflk6jvcjj2.fsf at helmutwaitzmann.news.arcor.de>:

|Junichi Uekawa <dancer at netfort.gr.jp> writes:

|>> >I have concerns wrt shell quoting,
|>> 
|>> Could you please explain more detailed?
|>
|>The implications of needing quoting means that previous 
|>quoting conventions will need to change.
|>
|>Applications which used to pass quoted text to su, and 
|>needs to quote differently now need to change dramatically.

This looks like a CON.  But it is a misunderstanding:  With the
transition to new su there is no change in whether or how quoting has to
be done, because old su as well as new su does not do any quoting.

Some people are seduced to the wrong assumptions about the ARGS
processing of old su by two (undocumented) features of old su:  It
concatenates a commandline out of the ARGS, and, if neither the '-c' nor
the '--command' OPTS are supplied, it inserts a '-c' option implicitly.

Noticing the positional ARGS rather than a commandline they suppose that
su doesn't construct a commandline but simply invokes a command.

For example:

$ touch '/tmp/;rm -r /'
$ su -- root ls -- /tmp/\;*

is supposed to do a

execl("/bin/ls", "ls", "--", "/tmp/;rm -r /", NULL)

runtime library call rather than a

execl("/bin/sh", "sh", "-c", "ls -- /tmp/;rm -r /", NULL)

runtime library call.

So, they don't know, that su will just collect the ARGS and concatenate
them with spaces in between as a commandline for the shell's invocation
option '-c'.  Maybe, they don't even know that su will call a shell (to
let it evaluate the commandline) rather than invoking the command by
itself.

Therefore they don't recognize the need for proper quoting of the ARGS to
be concatenated into the commandline that is evaluated by the shell
invoked by the second runtime library call above.

And it is interesting:  The wrong assumptions about quoting are made with
old su rather than with new su, as authors are aware of the need of
quoting, when they have to concatenate the ARGS for themselves.


With new su, there is no easy way to invoke that fatal old su command
from the example above.

$ su root -c 'ls -- /tmp/\;*'

which crosses one's mind first, won't do any harm (with both old and new
su) but is not accurately the same:  The filename pattern is expanded by
root's shell rather than by the invokers shell.  Whereas (only with new su)

$ su -- root -c 'ls -- ${1+"$@"}' sh /tmp/\;*

would have the intended semantics of the original command

$ su -- root ls -- /tmp/\;*

:  The filename pattern is expanded by the invokers' shell and the
filenames are passed as positional parameters to the root shell and can
be accessed by the '${1+"$@"}' construct.  No problem remains with
quoting and the filename '/tmp/;rm -r', as it is not part of the
commandline, can't do any harm.  It will not be evaluated, just passed
unchanged to the argument list of 'ls'.  This turns out to be a PRO.


>You all know that I'm too technically challenged for doing so.
>
>> With sarge just released, it looks like a perfect time to go for this
>> transition. C++ ABI transition seems faaar much intrusive (and somehow
>> discutable, but it must be my anti-c++ religious position) to me than this
>> *fix*.
>> 
>> I'd say that we need to summarize the rationnal and how to adapt packages to
>> the new, fixed behaviour on, say, 100 lines, post it to
>> debian-devel-announce under the title "su behaviour transition" or so, wait
>> 2 weeks, and upload a fixed version.

This is a draft of a su invocation adaption howto:

-------------- next part --------------
SU ADAPTION HOWTO:

This howto tells, how to adapt an old su invocation into one, that conforms
to old and new su's manual page and does not rely on the undocumented
features of old su (concatenating ARGS, use an implicit option '-c').

Of cause, the adaption will not change the semantics of the su invocation.

The adapted su invocation will then show the same behavior with old and new
su.

(0) In the su invocation that is to be converted to new su identify the
    elements which are shown in the synopsis:

    Quoting the manual page su(1):

       SYNOPSIS
	      su [OPTS] [-] [username [ARGS]]

    Note:  Arguments in brackets are optional.

    Examples ('$' is the shell prompt):

    $ su - root date

    There are no OPTS given.  A '-', the username 'root' and one
    ARGS, 'date' is supplied.

    $ su -- - root date -s now + 3 seconds

    There is one OPTS given: '--'.  A '-', the username 'root' and 6
    ARGS: 'date', '-s', 'now', '+', '3', and 'seconds', are
    supplied.

(1) Does the su invocation supply any ARGS?

    No ARGS?

       Then the current su invocation looks like this:

       su [OPTS] [-] [username]

       You are done.  There is nothing to do:  The semantics of this su
       invocation will remain the same with new su.

    One ARG?

       Then the current su invocation looks like this:

       su [OPTS] [-] username ONE_ARG

       Example:

       $ su - root date

       Continue with (2).

    More than one ARGS?

       So there are at least two ARGS.  The current su invocation looks
       like this:

       su [OPTS] [-] username ALL ARGS

       Example:

       $ su -- - root date -s now + 3 seconds

       Replace ALL ARGS by the concatenation of ALL ARGS with interjacent
       spaces into one long parameter ARG:

       su [OPTS] [-] username ARG

       Example:

       $ su -- - root date\ -s\ now\ +\ 3\ seconds

       (The backslashes are needed by the shell that evaluates this command,
       to pass to su one long ARG.)

       Continue with (2).

(2) There is exactly one ARG (maybe a concatenated one).  The current su
    invocation looks like this:

    su [OPTS] [-] username ARG

    Example:

    $ su -- - root date\ -s\ now\ +\ 3\ seconds

    (The backslashes are needed by the shell that evaluates this command,
    to pass to su one long ARG.)

    Continue with (3).

(3) Does the current su invocation supply the '-c' option?

    Yes?

       Then the current su invocation looks like this:

       su [NO_CMD_OPTS] -c COMMAND [NO_CMD_OPTS] [-] username ARG

       Example:

       $ su -c date -- - root -s\ now\ +\ 3\ seconds

       Remove the ARG from the tail of the command and append it to the
       COMMAND with an interjacent space:

       su [NO_CMD_OPTS] -c COMMAND_WITH_ARG [NO_CMD_OPTS] [-] username

       Example:

       $ su -c date\ -s\ now\ +\ 3\ seconds -- - root

       Note:  The second parameter of su, i.e. the parameter following
       '-c', is the whole string 'date -s now + 3 seconds' as one
       parameter.  The backslashes in the commandline prevent the shell
       from breaking that string apart in six components.

       You are done.  The semantics of this su invocation will remain the
       same with both old and new su.

    No?

       Continue with (5).

(5) Does the current su invocation supply the '--command=' option?

    Yes?

       Then the current su invocation looks like this:

       su [NO_CMD_OPTS] --command=COMMAND [NO_CMD_OPTS] [-] username ARG

       Example:

       $ su --command=date -- - root -s\ now\ +\ 3\ seconds

       Remove the ARG from the tail of the command and append it to the
       COMMAND with an interjacent space:

       su [NO_CMD_OPTS] --command=COMMAND_WITH_ARG [NO_CMD_OPTS] [-] username

       Example:

       $ su --command=date\ -s\ now\ +\ 3\ seconds -- - root

       Note:  The first parameter of su, i.e. the parameter starting with
       '--command=', comprises the whole string 'date -s now + 3 seconds'
       as one parameter.  The backslashes in the commandline prevent the
       shell from breaking that string apart in six components.

       You are done.  The semantics of this su invocation will remain the
       same with both old and new su.

    No?

       There is neither a '-c' nor a '--command=' option.  The current su
       invocation looks like this:

       su [NO_CMD_OPTS] [-] username ARG

       Example:

       $ su -- - root date\ -s\ now\ +\ 3\ seconds

       Remove the ARG from the end of the su argument list and insert a
       '-c' option with ARG as its parameter anywhere (but before the '--'
       option if there is one) into the parameter list of the su
       invocation:

       su -c ARG [NO_CMD_OPTS] [-] username

       Example:

       $ su -c date\ -s\ now\ +\ 3\ seconds -- - root

       Note:  The second parameter of su, i.e. the parameter following
       '-c', is the whole string 'date -s now + 3 seconds' as one
       parameter.  The backslashes in the commandline prevent the shell
       from breaking that string apart in six components.

       You are done.  The semantics of this su invocation will remain the
       same with both old and new su.

%% Emacs Local Variables:
%% Emacs coding: iso-8859-1
%% Emacs fill-column: 75
%% Emacs End:
-------------- next part --------------
       
>Sure. But not during August while many DD's are VAC (and not only
>French ones). We should make the announcement ASAP. Idealistically
>before I leave for holidays on Aug 1st for something like 3 weeks.
>
>This announcement would mention September 15th as target release date
>for the change, leaving people enough time to preventively adapt their
>work.
>
>The announcement must be as short as possible and be very clear bout
>the consequence. 

Is the su adaption howto above too long?  Is it clear enough?

>Deep technical details and rationale should be left to an appendix.

Part of that appendix should be a RTFM to the manual page bash(1):
namely the invocation options '-c' and '-s', and the section about
parameter expansion.
-- 
Wenn Sie mir E-Mail schreiben, stellen |  When writing me e-mail, please
Sie bitte vor meine E-Mail-Adresse     |  precede my e-mail address with
meinen Vor- und Nachnamen, etwa so:    |  my full name, like
Helmut Waitzmann <xxx at example.net>, (Helmut Waitzmann) xxx at example.net