Skip to content

Commit

Permalink
ssh/knownhosts: fix variable reuse bug in checkAddrs
Browse files Browse the repository at this point in the history
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 <bradfitz@golang.org>
Run-TryBot: Brad Fitzpatrick <bradfitz@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
  • Loading branch information
dsnet committed Apr 18, 2017
1 parent cbc3d08 commit efac7f2
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 5 deletions.
8 changes: 4 additions & 4 deletions ssh/knownhosts/knownhosts.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -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
}

Expand Down
2 changes: 1 addition & 1 deletion ssh/knownhosts/knownhosts_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit efac7f2

Please sign in to comment.