[Bash-completion-devel] [bash-completion-Bugs][311614] Quoting to prevent globbing (was: quoting bug in _known_hosts)

[bash-completion-bugs at alioth.debian.org]

Bugs item #311614, was changed at 2009-04-22 13:00 by Eric Blake
You can respond by visiting: 
https://alioth.debian.org/tracker/?func=detail&atid=413095&aid=311614&group_id=100114

Status: Open
Priority: 3
Submitted By: Eric Blake (eblake-guest)
Assigned to: Freddy Vulto (fvu-guest)
Summary: Quoting to prevent globbing (was: quoting bug in _known_hosts) 
Distribution: --Distribution-Agnostic--
Originally reported in: None
Milestone: 1.0
Status: None
Original bug number: 


Initial Comment:
There is a quoting bug in _known_hosts, which causes the shell to attempt glob expansion.  Although unlikely, a user can name a file to include shell metacharacters so that the glob performs arbitrary actions.

This portion of _known_hosts:

 COMPREPLY=($( awk 'BEGIN {FS=","}
                     /^\s*[^|\#]/ {for (i=1; i<=2; ++i) { \
                            gsub(" .*$", "", $i); \
                            if ($i ~ /'$cur'/) {print $i} \
                     }}' "${kh[@]}" 2>/dev/null ));

Needs "" around $cur.  Otherwise, something like 'ssh <tab>' causes cur to be defined as [a-z.], and since $cur does not occur in "", the shell treats it as a glob.


----------------------------------------------------------------------

Comment By: Eric Blake (eblake-guest)
Date: 2009-08-28 00:49

Message:
Here's a real-life example where suppressing the globbing actually matters:

http://cygwin.com/ml/cygwin/2009-04/msg00442.html

On cygwin, attempts to perform ANY glob, where the string being checked for potential expansion contains a literal backslash, issues a warning about using backslash rather than forward slash as a path separator.  Double-quoting $cur silences this warning.


----------------------------------------------------------------------

Comment By: Freddy Vulto (fvu-guest)
Date: 2009-08-27 20:57

Message:
Because this has survived for so many years in bash-completion, I don't think it's really an issue.  Especially considering the great lengths you need to go to make things go wrong:

    $ cur="[a-z.]"
    $ echo 'if ($i ~ /'$cur'/) {print $i}'  # (1)
    if ($i ~ /[a-z.]/) {print $i}           # (2)
    $ mkdir -p 'if ($i ~ /'a'/) {print $i}' # (3)
    $ echo 'if ($i ~ /'$cur'/) {print $i}'  # 
    if ($i ~ /a/) {print $i}                # (4)
    $ rm -rf 'if ($i ~ /'                   # Clean up

Explanation of the above: A sample awk-snippet (1) appears to echo all right (2), but creating awk-like dirs (3) however, causes the glob to resolve, resulting in a mangled awk-snippet (4): the awk-snippet resolves to `if ($i ~ /a/) {print $i}' where it should've been: `if ($i ~ /[a-z.]/) {print $i}'.

Also, quoting solves only half the globbing problem:

    $ cd /var
    $ cur="*"
    $ a=( $( echo "$cur" ) )    # (1)
    $ echo "${a[@]}"            # (2)
    backups cache crash games lib local lock log mail opt run spool tmp
    $

Explanation of the above: Additional globbing is performed when assigning items to `a' (1), causing `a' to contain globbing results instead of '*' (2).

Considering all this, for now I'd rather use quotes just where whitespace might occur, and not quote whole bash-completion for noglobbing sake: Let's just fix globbing where real bugs occur.

For future development however, I think it should be fixed.  Not by quoting but by using `set -o noglob'. Since most of bash-completion is not using globbing and just suffering its consequences, why don't we turn things upside-down and (within a completion) disable globbing by default?

Maybe we can do so after we branched development (drop bash-2 support, merge bash-completion-lib): if a user starts to complete, globbing is turned off, and at the end of a completion, globbing is restored to its previous setting.  Example:

    $ oldSetOptions=$(set +o)
    $ set -o noglob
    $ cur=*
    $ echo $cur
    *
    $ arr=( $( echo $cur ) )
    $ echo "${arr[@]}"
    *
    $ eval "$oldSetOptions" 2> /dev/null
    $

What do you all think?

Freddy Vulto
http://fvue.nl

----------------------------------------------------------------------

Comment By: Eric Blake (eblake-guest)
Date: 2009-05-18 15:52

Message:
I regenerated the patch against the latest git master; with many more instances of underquoted $cur fixed in bash_completion proper.  However, I suspect that a full audit of the contrib files will find yet more underquoted instances.

----------------------------------------------------------------------

Comment By: Eric Blake (eblake-guest)
Date: 2009-04-22 22:08

Message:
attaching a patch for all instances of underquoted $cur that I could find


----------------------------------------------------------------------

You can respond by visiting: 
https://alioth.debian.org/tracker/?func=detail&atid=413095&aid=311614&group_id=100114