From 382ba4b137b42d5f25a7e256bb7c053ee5ac7b64 Mon Sep 17 00:00:00 2001 From: Werner Koch Date: Wed, 28 Jan 2015 20:32:28 +0100 Subject: [PATCH] gpg: Limit the size of key packets to a sensible value. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * g10/parse-packet.c (MAX_KEY_PACKET_LENGTH): New. (MAX_UID_PACKET_LENGTH): New. (MAX_COMMENT_PACKET_LENGTH): New. (MAX_ATTR_PACKET_LENGTH): New. (parse_key): Limit the size of a key packet to 256k. (parse_user_id): Use macro for the packet size limit. (parse_attribute): Ditto. (parse_comment): Ditto. -- Without that it is possible to force gpg to allocate large amounts of memory by using a bad encoded MPI. This would be an too easy DoS. Another way to mitigate would be to change the MPI read function to allocate memory dynamically while reading the MPI. However, that complicates and possibly slows down the code. A too large key packet is in any case a sign for broken data and thus gpg should not use it. Reported-by: Hanno Böck GnuPG-bug-id: 1823 Signed-off-by: Werner Koch --- g10/parse-packet.c | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/g10/parse-packet.c b/g10/parse-packet.c index 73c4434c1..524fabee1 100644 --- a/g10/parse-packet.c +++ b/g10/parse-packet.c @@ -35,6 +35,14 @@ #include "main.h" #include "i18n.h" + +/* Maximum length of packets to avoid excessive memory allocation. */ +#define MAX_KEY_PACKET_LENGTH (256 * 1024) +#define MAX_UID_PACKET_LENGTH ( 2 * 1024) +#define MAX_COMMENT_PACKET_LENGTH ( 64 * 1024) +#define MAX_ATTR_PACKET_LENGTH ( 16 * 1024*1024) + + static int mpi_print_mode; static int list_mode; static estream_t listfp; @@ -1983,6 +1991,14 @@ parse_key (IOBUF inp, int pkttype, unsigned long pktlen, err = gpg_error (GPG_ERR_INV_PACKET); goto leave; } + else if (pktlen > MAX_KEY_PACKET_LENGTH) + { + log_error ("packet(%d) too large\n", pkttype); + if (list_mode) + es_fputs (":key packet: [too larget]\n", listfp); + err = gpg_error (GPG_ERR_INV_PACKET); + goto leave; + } timestamp = read_32 (inp); pktlen -= 4; @@ -2415,7 +2431,7 @@ parse_user_id (IOBUF inp, int pkttype, unsigned long pktlen, PACKET * packet) allocatable, and a very large pktlen could actually cause our allocation to wrap around in xmalloc to a small number. */ - if (pktlen > 2048) + if (pktlen > MAX_UID_PACKET_LENGTH) { log_error ("packet(%d) too large\n", pkttype); if (list_mode) @@ -2496,7 +2512,7 @@ parse_attribute (IOBUF inp, int pkttype, unsigned long pktlen, /* We better cap the size of an attribute packet to make DoS not too easy. 16MB should be more then enough for one attribute packet (ie. a photo). */ - if (pktlen > 16*1024*1024) + if (pktlen > MAX_ATTR_PACKET_LENGTH) { log_error ("packet(%d) too large\n", pkttype); if (list_mode) @@ -2540,7 +2556,7 @@ parse_comment (IOBUF inp, int pkttype, unsigned long pktlen, PACKET * packet) overflow in the malloc below. Comment packets are actually not anymore define my OpenPGP and we even stopped to use our private comment packet. */ - if (pktlen > 65536) + if (pktlen > MAX_COMMENT_PACKET_LENGTH) { log_error ("packet(%d) too large\n", pkttype); if (list_mode)