From 983f7b2acbd1e7580652bbeb0c3d64f9dd19d9e4 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Thu, 15 Mar 2018 08:44:52 +0100 Subject: gpg: Fix out-of-bound read in subpacket enumeration * g10/parse-packet.c (enum_sig_subpkt): Check buflen before reading the type octet. Print diagnostic. -- If the final subpacket has only a length header evaluating to zero and missing the type octet, a read could happen right behind the buffer. Valgrind detected this. Fix is obvious. Note that the further parsing of the subpacket is still okay because it always checks the length. Note further that --list-packets uses a different code path and already reported an error. Reported-by: Philippe Antoine He provided a test file copied below. Running "gpg -v --verify" on it triggered the bug. -----BEGIN PGP ARMORED FILE----- Comment: Use "gpg --dearmor" for unpacking kA0DAAoBov87N383R0QBrQJhYgZsb2wucHlaqTVWYnl0ZXMgPSBbMHg1LCAweDY0 LCAweDRjLCAweGM0LCAweDMsIDB4MCwgMHg0LCAweDAsIDB4YWMsIDB4YSwgMHhj MSwgMHhjMSwgMHgyLCAweDEsIDB4MiwgMHg3LCAweDQwLCAweDIsIDB4MiwgMHgy LCAweDIsIDB4MiwgMHgyLCAweDIsIDB4MiwgMHgyLCAweDJkLCAweGRkLCAweDIs IDB4MiwgMHgyLCAweDIsIDB4MiwgMHgyLCAweDIsIDB4MiwgMHgyLCAweDIsIDB4 MiwgMHgyLCAweDIsIDB4MiwgMHgyLCAweDIsIDB4NzcsIDB4ODcsIDB4MiwgMHgy LCAweDIsIDB4MiwgMHgyLCAweDIsIDB4MiwgMHgyLCAweDIsIDB4MiwgMHgyLCAw eDIsIDB4MiwgMHgyLCAweDIsIDB4MiwgMHg3NywgMHg4NywgMHgyLCAweDIsIDB4 MiwgMHgyLCAweDIsIDB4MiwgMHgyLCAweDIsIDB4MiwgMHgyLCAweDIsIDB4Miwg MHgyLCAweDIsIDB4MiwgMHgyLCAweDc3LCAweDg3LCAweDIsIDB4MiwgMHgyLCAw eDIsIDB4MiwgMHgyLCAweDIsIDB4MCwgMHhhZF0KCmZvciBpIGluIHJhbmdlKGxl bihieXRlcykpOgogICAgaWYgaSUxNiA9PSAwOgogICAgICAgIHByaW50CiAgICAg ICAgcHJpbnQgIiUwNngiICUgaSwKICAgIHByaW50ICIlMDJ4ICIgJSBieXRlc1tp XSwKiQJNBAABCgAeFiEEU+Y3aLjDLA3x+9Epov87N383R0QFAlqpNVYAAAoJEKL/ Ozd/N0dElccP/jBAcFHyeMl7kop71Q7/5NPu3DNULmdUzOZPle8PVjNURT4PSELF qpJ8bd9PAsO4ZkUGwssY4Kfb1iG5cR/a8ADknNd0Cj9/QA2KMVNmgYtReuttAjvn hQRm2VY0tvDCVAPI/z8OnV/NpOcbk8kSwE+shLsP7EwqL5MJNMXKqzm1uRxGNYxr 8TNuECo3DO64O2NZLkMDXqq6lg+lSxvDtXKxzKXgVC+GMtOE56lDwxWLqr39f9Ae Pn0q2fVBKhJfpUODeEbYSYUp2hhmMUIJL/ths9MvyRZ9Z/bHCseFPT58Pgx6g+MP q+iHnVZEIVb38XG+rTYW9hvctkRZP/azhpa7eO8JAZuFNeBGr4IGapwzFPvQSF4B wBXBu0+PPrV9VJVe98P4nw2xcuJmkn6mgZhRVYSqDIhY64bSTgQxb/pdtGwrTjtL WoUKVI+joLRPnDmwexH9+QJCB+uA6RsN/LqsQfDseyr40Z6dHJRqWGgP3ll6iZgw WF768uiIDJD8d4fegVnkpcH98Hm0I/dKsMR1MGV/sBxYC8mAOcOWwSPNGDqPlwwR eWPdr1O6CoYEWwiZMicSe0b5TsjB5nkAWMy7c9RyhtMJzCQ/hFpycpj0A0Zs+OGa eJQMZZV0s8AQZ04JzoX0zRpe0RcTyJn3Tr6QGbVi9tr+QdKHFuDMUqoX =qYZP -----END PGP ARMORED FILE----- Signed-off-by: Werner Koch --- g10/parse-packet.c | 9 +++++++++ 1 file changed, 9 insertions(+) (limited to 'g10/parse-packet.c') diff --git a/g10/parse-packet.c b/g10/parse-packet.c index eee14f64e..db4e3ab69 100644 --- a/g10/parse-packet.c +++ b/g10/parse-packet.c @@ -1702,6 +1702,8 @@ enum_sig_subpkt (const subpktarea_t * pktbuf, sigsubpkttype_t reqtype, } if (buflen < n) goto too_short; + if (!buflen) + goto no_type_byte; type = *buffer; if (type & 0x80) { @@ -1776,6 +1778,13 @@ enum_sig_subpkt (const subpktarea_t * pktbuf, sigsubpkttype_t reqtype, if (start) *start = -1; return NULL; + + no_type_byte: + if (opt.verbose) + log_info ("type octet missing in subpacket\n"); + if (start) + *start = -1; + return NULL; } -- cgit v1.2.3 From 34ec0125614674725cb78cf9f8d5e6b0f8c64064 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 21 Mar 2018 19:45:31 +0100 Subject: doc: Typo fix in comment. -- --- g10/parse-packet.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'g10/parse-packet.c') diff --git a/g10/parse-packet.c b/g10/parse-packet.c index db4e3ab69..a64d4f723 100644 --- a/g10/parse-packet.c +++ b/g10/parse-packet.c @@ -964,10 +964,10 @@ skip_packet (IOBUF inp, int pkttype, unsigned long pktlen, int partial) } -/* Read PKTLEN bytes form INP and return them in a newly allocated - buffer. In case of an error (including reading fewer than PKTLEN - bytes from INP before EOF is returned), NULL is returned and an - error message is logged. */ +/* Read PKTLEN bytes from INP and return them in a newly allocated + * buffer. In case of an error (including reading fewer than PKTLEN + * bytes from INP before EOF is returned), NULL is returned and an + * error message is logged. */ static void * read_rest (IOBUF inp, size_t pktlen) { -- cgit v1.2.3