From 24ffb5feb3312a39054178a4b0a4554fc2201248 Mon Sep 17 00:00:00 2001 From: KB Sriram Date: Wed, 6 May 2015 21:55:27 -0700 Subject: [PATCH] x/crypto/openpgp: Limit packet recursion depth. A carefully crafted packet can cause the packet reader to push an infinite number of recursive packet readers. This change limits the number of recursive parsing levels within the packet reader. More details at: http://mumble.net/~campbell/misc/pgp-quine https://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2013-4402 Fixes #10751 Change-Id: Ib4e102d85f6496e2c7feb5b9d7e5db45db6032df Reviewed-on: https://go-review.googlesource.com/9843 Reviewed-by: Adam Langley --- openpgp/packet/reader.go | 18 ++++++++++++++++-- openpgp/read.go | 8 ++++++-- openpgp/read_test.go | 21 +++++++++++++++++++++ 3 files changed, 43 insertions(+), 4 deletions(-) diff --git a/openpgp/packet/reader.go b/openpgp/packet/reader.go index 753bf8f..34bc7c6 100644 --- a/openpgp/packet/reader.go +++ b/openpgp/packet/reader.go @@ -16,6 +16,14 @@ type Reader struct { readers []io.Reader } +// New io.Readers are pushed when a compressed or encrypted packet is processed +// and recursively treated as a new source of packets. However, a carefully +// crafted packet can trigger an infinite recursive sequence of packets. See +// http://mumble.net/~campbell/misc/pgp-quine +// https://web.nvd.nist.gov/view/vuln/detail?vulnId=CVE-2013-4402 +// This constant limits the number of recursive packets that may be pushed. +const maxReaders = 32 + // Next returns the most recently unread Packet, or reads another packet from // the top-most io.Reader. Unknown packet types are skipped. func (r *Reader) Next() (p Packet, err error) { @@ -44,9 +52,15 @@ func (r *Reader) Next() (p Packet, err error) { // Push causes the Reader to start reading from a new io.Reader. When an EOF // error is seen from the new io.Reader, it is popped and the Reader continues -// to read from the next most recent io.Reader. -func (r *Reader) Push(reader io.Reader) { +// to read from the next most recent io.Reader. Push returns a StructuralError +// if pushing the reader would exceed the maximum recursion level, otherwise it +// returns nil. +func (r *Reader) Push(reader io.Reader) (err error) { + if len(r.readers) >= maxReaders { + return errors.StructuralError("too many layers of packets") + } r.readers = append(r.readers, reader) + return nil } // Unread causes the given Packet to be returned from the next call to Next. diff --git a/openpgp/read.go b/openpgp/read.go index c50a509..dff65a8 100644 --- a/openpgp/read.go +++ b/openpgp/read.go @@ -211,7 +211,9 @@ FindKey: } md.decrypted = decrypted - packets.Push(decrypted) + if err := packets.Push(decrypted); err != nil { + return nil, err + } return readSignedMessage(packets, md, keyring) } @@ -235,7 +237,9 @@ FindLiteralData: } switch p := p.(type) { case *packet.Compressed: - packets.Push(p.Body) + if err := packets.Push(p.Body); err != nil { + return nil, err + } case *packet.OnePassSignature: if !p.IsLast { return nil, errors.UnsupportedError("nested signatures") diff --git a/openpgp/read_test.go b/openpgp/read_test.go index db805f0..03bf5d8 100644 --- a/openpgp/read_test.go +++ b/openpgp/read_test.go @@ -135,6 +135,25 @@ func TestTextSignedMessage(t *testing.T) { checkSignedMessage(t, signedTextMessageHex, signedTextInput) } +// The reader should detect "compressed quines", which are compressed +// packets that expand into themselves and cause an infinite recursive +// parsing loop. +// The packet in this test case comes from Taylor R. Campbell at +// http://mumble.net/~campbell/misc/pgp-quine/ +func TestCampbellQuine(t *testing.T) { + md, err := ReadMessage(readerFromHex(campbellQuine), nil, nil, nil) + if md != nil { + t.Errorf("Reading a compressed quine should not return any data: %#v", md) + } + structural, ok := err.(errors.StructuralError) + if !ok { + t.Fatalf("Unexpected class of error: %T", err) + } + if !strings.Contains(string(structural), "too many layers of packets") { + t.Fatalf("Unexpected error: %s", err) + } +} + var signedEncryptedMessageTests = []struct { keyRingHex string messageHex string @@ -440,3 +459,5 @@ const dsaKeyWithSHA512 = `9901a2044f04b07f110400db244efecc7316553ee08d179972aab8 const unknownHashFunctionHex = `8a00000040040001990006050253863c24000a09103b4fe6acc0b21f32ffff01010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101` const missingHashFunctionHex = `8a00000040040001030006050253863c24000a09103b4fe6acc0b21f32ffff01010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101010101` + +const campbellQuine = `a0b001000300fcffa0b001000d00f2ff000300fcffa0b001000d00f2ff8270a01c00000500faff8270a01c00000500faff000500faff001400ebff8270a01c00000500faff000500faff001400ebff428821c400001400ebff428821c400001400ebff428821c400001400ebff428821c400001400ebff428821c400000000ffff000000ffff000b00f4ff428821c400000000ffff000000ffff000b00f4ff0233214c40000100feff000233214c40000100feff0000`