[Bash-completion-devel] [bash-completion-Bugs][311595] tries to execute ssh UserKnownHostsFile

bash-completion-bugs at alioth.debian.org bash-completion-bugs at alioth.debian.org
Thu Apr 16 21:04:57 UTC 2009


Bugs item #311595, was changed at 2009-04-11 04:30 by Freddy Vulto
You can respond by visiting: 
https://alioth.debian.org/tracker/?func=detail&atid=413095&aid=311595&group_id=100114

Status: Open
Priority: 3
Submitted By: Olivier Crête (tester-guest)
Assigned to: Freddy Vulto (fvu-guest)
Summary: tries to execute ssh UserKnownHostsFile 
Distribution: Gentoo
Originally reported in: None
Milestone: None
Status: None
Original bug number: 


Initial Comment:
The _known_hosts function tries to execute the UserKnownHostsFile file instead of just getting its name, there is an extra "eval", if you just remove it, it works fine.

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

>Comment By: Freddy Vulto (fvu-guest)
Date: 2009-04-16 23:04

Message:
Patch looks good, I'll apply.

Since the patch already uses parameter expansion to remove double quotes, the `eval' can be removed as well.

Single quotes aren't allowed around a UserKnownHostsFile parameter (tested on OpenSSH_5.1p1 Debian-3ubuntu1, OpenSSL 0.9.8g 19 Oct 2007) so removing the double quotes should be enough.

Theoretically, a specified UserKnownHostsFile could contain double quotes.  So an improvement still would be to really dequote instead of search-replacing all quotes.  An improved `dequote()' should do without `eval' and instead accept an additional parameter $2:

---8<---- (pseudocode) -------------------------------------------------
# $1 mixed  Param to dequote
# $2 integer  Quotes to remove:
#             0 (default)  Remove single or double quotes
#             1  Remove single quotes
#             2  Remove double quotes
dequote() {
    if (($2 == 0/1) && ($1 =~ "[\s]'$1'[\s]")) {
	trim whitespace & remove single quotes
    } elseif (($2 == 0/2) && ($1 =~ '[\s]"$1"[\s]')) {
	trim whitespace & remove double quotes
    fi
}
---8<-------------------------------------------------------------------

Freddy

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

Comment By: David Paleino (hanska-guest)
Date: 2009-04-15 14:40

Message:
A Debian user reported this bug too, and he attached a patch. Freddy, since you've reassigned the bug to yourself, would you mind taking a look?

http://bugs.debian.org/cgi-bin/bugreport.cgi?msg=5;filename=support_multiple_hosts_files.patch;att=1;bug=524190

David

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

Comment By: David Paleino (hanska-guest)
Date: 2009-04-13 23:45

Message:
Aha, maybe re-reading the bugreport I didn't fully understand it. Yes, the quotes are really needed. However, what's the probability of having parameter expansion inside ~/.ssh/config (or whatever)?

Freddy, feel free to reassign this bug to yourself if you wish. In the meanwhile, I'm reopening it.

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

Comment By: Freddy Vulto (fvu-guest)
Date: 2009-04-13 23:35

Message:
The `eval' is/was used - and is still necessary - to remove possible quotes around the UserKnownHostsFiles... ("known  hosts{1,2}" in my previous message).

Maybe we should remove these quotes with parameter expansion instead of  'eval' then?

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

Comment By: Freddy Vulto (fvu-guest)
Date: 2009-04-13 23:17

Message:
The quote around the sed output is still necessary if we want to support the `known_hosts' files containing whitespace, for example with this ssh config:

    UserKnownHostsFile "known  hosts1"  # Contains TWO spaces
    UserKnownHostsFile "known  hosts2"  # Contains TWO spaces

So I'm adding the quotes again around the sed output since removing 'eval' alone already fixes the execution(!!!) :-(

    $ echo $( sed -ne 's/^[ \t]*UserKnownHostsFile['"$'\t '"']*\(.*\)$/\1/p' config )
    "known hosts1" "known hosts2"
    $ echo "$( sed -ne 's/^[ \t]*UserKnownHostsFile['"$'\t '"']*\(.*\)$/\1/p' config )"
    "known  hosts1"
    "known  hosts2"

A TODO is to make ssh completion actually handle the case of more than one specified UserKnownHostsFile (didn't even know it was supported by ssh): `user_kh' and `global_kh' have to become arrays.

So we have to watch for all `eval echo'-s then?  There's an "eval echo" in the `dequote()' function too.  We have to make sure all parameters to `dequote' are quoted (as is done in _ssh()), otherwise there's potentially the same problem(?):

    $ dequote "$'foo\nbar'"
    foo
    bar
    $ dequote $'foo\nbar'
    foo
    bash: bar: command not found


Regards,
Freddy Vulto

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

Comment By: David Paleino (hanska-guest)
Date: 2009-04-13 17:11

Message:
commit a9994ac15f29892a27ebfff6ded63930a43f1187
Author: David Paleino <d.paleino at gmail.com>
Date:   Mon Apr 13 17:10:41 2009 +0200

    Remove eval() and sed quoting in _known_hosts() (Alioth: #311595)
    
        Fixes execution (!!!) of hosts specified by {Global,User}KnownHosts
        in SSH config files.

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

Comment By: David Paleino (hanska-guest)
Date: 2009-04-13 17:07

Message:
Hello,
I clearly remember being able to reproduce the bug at the time. However, I'm not able anymore to reproduce it, and Debian #504650 doesn't show up anymore, both with and without quotes.

I've been able to reproduce *this* bug (I tried a mixture of this and Debian #504650, i.e. multiple-spaced hosts on more lines) and seems to work without quotes.

Also, I believe those two eval's are not needed at all -- and they cause "execution" of specified hostnames. Thus, I'm fixing this bug -- Freddy, if you have more to say, please reopen this bug and feel free to revert my commit (will arrive in the next minutes).

Ciao,
David

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

Comment By: Ville Skyttä  (scop-guest)
Date: 2009-04-13 11:10

Message:
Ok, I can reproduce now.  It also appears to work with the eval if the quotes around "$( sed ... )" are removed, but those were added in http://git.debian.org/?p=bash-completion/bash-completion.git;a=commitdiff;h=fec084a69be9383776090df6facfea1a790c5c66 to fix http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=504650

I was not able to reproduce the issue in Debian bug 504650 (works for me both with and without the quotes) but then again it is missing an exact, concrete reproducer so it could be that I'm not testing the right thing.  Freddy, David?

I'm not sure if the eval is needed either, but someone who understands Debian bug 504650 should check how removing the eval affects the scenario in it.

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

Comment By: Olivier Crête (tester-guest)
Date: 2009-04-13 00:07

Message:
The difference is that I have two UserKnownHost files in different "Host" sections... So there is a newline in between...
Simple example:
aa=$(eval echo "$(echo /dev/null; echo /dev/null)")

So bashcomp has to be able to deal with multiple answers here, just removing the eval seems to work.. I don't think its required anyway, but maybe I'm missing something (I'm not  a bash expert).

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

Comment By: Ville Skyttä  (scop-guest)
Date: 2009-04-12 23:40

Message:
I can't reproduce - the two eval lines in _known_hosts are both like
    foo=$( eval echo bar )
which for me does not try to execute "bar".

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

Comment By: Olivier Crête (tester-guest)
Date: 2009-04-12 23:03

Message:
Its 1.0.. and I also checked the git master (but didn't test it).

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

Comment By: Ville Skyttä  (scop-guest)
Date: 2009-04-12 18:47

Message:
Which version of bash_completion is this?  If not 1.0, please test with it.

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

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



More information about the Bash-completion-devel mailing list