On Sat, Nov 11, 2017 at 08:03:36PM +0000, Robert Peichaer wrote:
> On Sat, Nov 11, 2017 at 08:11:25PM +0100, Klemens Nanni wrote:
> > pre_path()ing directories with spaces is broken due to bad quoting.
> > 
> > This diff takes care of that by properly passing double quotes through
> > eval and quoting the arguments for no_path() individually.
> > 
> > Feedback?
> 
> What is actually broken?
> Can you give examples?
Sure, pardon me.

        $ typeset -f add_path
        function add_path {
            [[ -d ${1:-.} ]] && no_path $* && eval ${2:-PATH}="\$${2:-PATH}:$1"
        }
        $ echo $PATH
        /usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin
        $ mkdir some\ bin
        $ add_path some\ bin
        add_path: bin: not found
        $ echo $PATH
        /usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin
        $ PATH=$PATH:some\ bin
        $ del_path some\ bin
        $ echo $?
        0
        $ echo $PATH
        /usr/local/bin:/usr/bin:/bin:/usr/local/sbin:/usr/sbin:/sbin:some bin/

Since the double quotes are not escaped and thus quote the string to be
evaluated itself, `PATH=$PATH:some bin' is eventually being executed as
seen above.

pre_path() behaves the same trying to execute `PATH=some bin:$PATH'.

del_path() silently fails to remove the directory.

Here's another example showing how to exploit this:

        $ ls foo
        ls: foo: No such file or directory
        $ mkdir '. touch foo'
        $ add_path '. touch foo'
        $ ls foo
        foo

With my patch behaviour will be as expected.

Reply via email to