From efac7f277b17c19894091e358c6130cb6bd51117 Mon Sep 17 00:00:00 2001 From: Joe Tsai Date: Mon, 17 Apr 2017 18:50:50 -0700 Subject: [PATCH] ssh/knownhosts: fix variable reuse bug in checkAddrs Consider the following code: var p *int a := []int{0, 1, 2, 3} for _, i := range a { if i == 1 { p = &i } } fmt.Println(*p) // Prints 3 This prints 3 because the variable i is the exact same variable across all iterations of the loop. When the address is taken for some specific iteration, the user's intent is to capture the value of i at that given loop, but instead the value of i in the last loop is what remains. A bug this sort occurs in the check logic since the address of the knownKey is taken, but is changed upon subsequent iterations of the loop (which happens when there are multiple lines). Change-Id: Ic626778cdcde3968dcff4fa5e7206274957dcb04 Reviewed-on: https://go-review.googlesource.com/40937 Reviewed-by: Brad Fitzpatrick Run-TryBot: Brad Fitzpatrick TryBot-Result: Gobot Gobot --- ssh/knownhosts/knownhosts.go | 8 ++++---- ssh/knownhosts/knownhosts_test.go | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/ssh/knownhosts/knownhosts.go b/ssh/knownhosts/knownhosts.go index fe9768a..66bb425 100644 --- a/ssh/knownhosts/knownhosts.go +++ b/ssh/knownhosts/knownhosts.go @@ -367,19 +367,19 @@ func (db *hostKeyDB) checkAddrs(addrs []addr, remoteKey ssh.PublicKey) error { // hostname? // Algorithm => key. - knownKeys := map[string]*KnownKey{} + knownKeys := map[string]KnownKey{} for _, l := range db.lines { if l.match(addrs) { typ := l.knownKey.Key.Type() if _, ok := knownKeys[typ]; !ok { - knownKeys[typ] = &l.knownKey + knownKeys[typ] = l.knownKey } } } keyErr := &KeyError{} for _, v := range knownKeys { - keyErr.Want = append(keyErr.Want, *v) + keyErr.Want = append(keyErr.Want, v) } // Unknown remote host. @@ -389,7 +389,7 @@ func (db *hostKeyDB) checkAddrs(addrs []addr, remoteKey ssh.PublicKey) error { // If the remote host starts using a different, unknown key type, we // also interpret that as a mismatch. - if known := knownKeys[remoteKey.Type()]; known == nil || !keyEq(known.Key, remoteKey) { + if known, ok := knownKeys[remoteKey.Type()]; !ok || !keyEq(known.Key, remoteKey) { return keyErr } diff --git a/ssh/knownhosts/knownhosts_test.go b/ssh/knownhosts/knownhosts_test.go index cffd72a..2d0834b 100644 --- a/ssh/knownhosts/knownhosts_test.go +++ b/ssh/knownhosts/knownhosts_test.go @@ -141,7 +141,7 @@ func TestIPv6Address(t *testing.T) { } func TestBasic(t *testing.T) { - str := fmt.Sprintf("#comment\n\nserver.org,%s %s", testAddr, edKeyStr) + str := fmt.Sprintf("#comment\n\nserver.org,%s %s\notherhost %s", testAddr, edKeyStr, ecKeyStr) db := testDB(t, str) if err := db.check("server.org:22", testAddr, edKey); err != nil { t.Errorf("got error %q, want none", err)