From 5770296d904e90f15f38f77dfc2e43fdf5efc083 Mon Sep 17 00:00:00 2001 From: Roland Shoemaker Date: Tue, 9 Nov 2021 11:45:57 -0800 Subject: [PATCH] ssh: don't assume packet plaintext size When reading GCM and ChaChaPoly1305 packets, don't make assumptions about the size of the enciphered plaintext. This fixes two panics caused by standards non-compliant malformed packets. Thanks to Rod Hynes, Psiphon Inc. for reporting this issue. Fixes golang/go#49932 Fixes CVE-2021-43565 Change-Id: I660cff39d197e0d04ec44d11d792b22d954df2ef Reviewed-on: https://team-review.git.corp.google.com/c/golang/go-private/+/1262659 Reviewed-by: Katie Hockman Reviewed-by: Julie Qiu Reviewed-on: https://go-review.googlesource.com/c/crypto/+/368814 Trust: Roland Shoemaker Trust: Katie Hockman Run-TryBot: Roland Shoemaker TryBot-Result: Gopher Robot Reviewed-by: Julie Qiu Reviewed-by: Katie Hockman --- ssh/cipher.go | 8 ++++ ssh/cipher_test.go | 100 +++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 108 insertions(+) diff --git a/ssh/cipher.go b/ssh/cipher.go index bddbde5dbd..f8bdf4984c 100644 --- a/ssh/cipher.go +++ b/ssh/cipher.go @@ -394,6 +394,10 @@ func (c *gcmCipher) readCipherPacket(seqNum uint32, r io.Reader) ([]byte, error) } c.incIV() + if len(plain) == 0 { + return nil, errors.New("ssh: empty packet") + } + padding := plain[0] if padding < 4 { // padding is a byte, so it automatically satisfies @@ -710,6 +714,10 @@ func (c *chacha20Poly1305Cipher) readCipherPacket(seqNum uint32, r io.Reader) ([ plain := c.buf[4:contentEnd] s.XORKeyStream(plain, plain) + if len(plain) == 0 { + return nil, errors.New("ssh: empty packet") + } + padding := plain[0] if padding < 4 { // padding is a byte, so it automatically satisfies diff --git a/ssh/cipher_test.go b/ssh/cipher_test.go index 70a2b5b5cd..610982807d 100644 --- a/ssh/cipher_test.go +++ b/ssh/cipher_test.go @@ -8,7 +8,12 @@ import ( "bytes" "crypto" "crypto/rand" + "encoding/binary" + "io" "testing" + + "golang.org/x/crypto/chacha20" + "golang.org/x/crypto/internal/poly1305" ) func TestDefaultCiphersExist(t *testing.T) { @@ -129,3 +134,98 @@ func TestCBCOracleCounterMeasure(t *testing.T) { lastRead = bytesRead } } + +func TestCVE202143565(t *testing.T) { + tests := []struct { + cipher string + constructPacket func(packetCipher) io.Reader + }{ + { + cipher: gcmCipherID, + constructPacket: func(client packetCipher) io.Reader { + internalCipher := client.(*gcmCipher) + b := &bytes.Buffer{} + prefix := [4]byte{} + if _, err := b.Write(prefix[:]); err != nil { + t.Fatal(err) + } + internalCipher.buf = internalCipher.aead.Seal(internalCipher.buf[:0], internalCipher.iv, []byte{}, prefix[:]) + if _, err := b.Write(internalCipher.buf); err != nil { + t.Fatal(err) + } + internalCipher.incIV() + + return b + }, + }, + { + cipher: chacha20Poly1305ID, + constructPacket: func(client packetCipher) io.Reader { + internalCipher := client.(*chacha20Poly1305Cipher) + b := &bytes.Buffer{} + + nonce := make([]byte, 12) + s, err := chacha20.NewUnauthenticatedCipher(internalCipher.contentKey[:], nonce) + if err != nil { + t.Fatal(err) + } + var polyKey, discardBuf [32]byte + s.XORKeyStream(polyKey[:], polyKey[:]) + s.XORKeyStream(discardBuf[:], discardBuf[:]) // skip the next 32 bytes + + internalCipher.buf = make([]byte, 4+poly1305.TagSize) + binary.BigEndian.PutUint32(internalCipher.buf, 0) + ls, err := chacha20.NewUnauthenticatedCipher(internalCipher.lengthKey[:], nonce) + if err != nil { + t.Fatal(err) + } + ls.XORKeyStream(internalCipher.buf, internalCipher.buf[:4]) + if _, err := io.ReadFull(rand.Reader, internalCipher.buf[4:4]); err != nil { + t.Fatal(err) + } + + s.XORKeyStream(internalCipher.buf[4:], internalCipher.buf[4:4]) + + var tag [poly1305.TagSize]byte + poly1305.Sum(&tag, internalCipher.buf[:4], &polyKey) + + copy(internalCipher.buf[4:], tag[:]) + + if _, err := b.Write(internalCipher.buf); err != nil { + t.Fatal(err) + } + + return b + }, + }, + } + + for _, tc := range tests { + mac := "hmac-sha2-256" + + kr := &kexResult{Hash: crypto.SHA1} + algs := directionAlgorithms{ + Cipher: tc.cipher, + MAC: mac, + Compression: "none", + } + client, err := newPacketCipher(clientKeys, algs, kr) + if err != nil { + t.Fatalf("newPacketCipher(client, %q, %q): %v", tc.cipher, mac, err) + } + server, err := newPacketCipher(clientKeys, algs, kr) + if err != nil { + t.Fatalf("newPacketCipher(client, %q, %q): %v", tc.cipher, mac, err) + } + + b := tc.constructPacket(client) + + wantErr := "ssh: empty packet" + _, err = server.readCipherPacket(0, b) + if err == nil { + t.Fatalf("readCipherPacket(%q, %q): didn't fail with empty packet", tc.cipher, mac) + } else if err.Error() != wantErr { + t.Fatalf("readCipherPacket(%q, %q): unexpected error, got %q, want %q", tc.cipher, mac, err, wantErr) + } + } +}