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`