1
0
mirror of git://git.gnupg.org/gnupg.git synced 2024-06-02 22:38:02 +02:00

gpg: Fix bug with deeply nested compressed packets.

* g10/mainproc.c (MAX_NESTING_DEPTH): New.
(proc_compressed): Return an error code.
(check_nesting): New.
(do_proc_packets): Check packet nesting depth.  Handle errors from
check_compressed.

Signed-off-by: Werner Koch <wk@gnupg.org>
This commit is contained in:
Werner Koch 2013-10-02 09:11:43 +02:00
parent 801ea11f21
commit cd1b696b28
2 changed files with 45 additions and 9 deletions

2
NEWS
View File

@ -1,6 +1,8 @@
Noteworthy changes in version 2.0.22 (unreleased) Noteworthy changes in version 2.0.22 (unreleased)
------------------------------------------------- -------------------------------------------------
* Fixed bug with deeply nested compressed packets.
Noteworthy changes in version 2.0.21 (2013-08-19) Noteworthy changes in version 2.0.21 (2013-08-19)
------------------------------------------------- -------------------------------------------------

View File

@ -1,6 +1,7 @@
/* mainproc.c - handle packets /* mainproc.c - handle packets
* Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007, * Copyright (C) 1998, 1999, 2000, 2001, 2002, 2003, 2004, 2005, 2006, 2007,
* 2008, 2009 Free Software Foundation, Inc. * 2008, 2009 Free Software Foundation, Inc.
* Copyright (C) 2013 Werner Koch
* *
* This file is part of GnuPG. * This file is part of GnuPG.
* *
@ -42,6 +43,11 @@
#include "pka.h" #include "pka.h"
/* Put an upper limit on nested packets. The 32 is an arbitrary
value, a much lower should actually be sufficient. */
#define MAX_NESTING_DEPTH 32
struct kidlist_item { struct kidlist_item {
struct kidlist_item *next; struct kidlist_item *next;
u32 kid[2]; u32 kid[2];
@ -764,7 +770,7 @@ proc_encrypt_cb( IOBUF a, void *info )
return proc_encryption_packets( info, a ); return proc_encryption_packets( info, a );
} }
static void static int
proc_compressed( CTX c, PACKET *pkt ) proc_compressed( CTX c, PACKET *pkt )
{ {
PKT_compressed *zd = pkt->pkt.compressed; PKT_compressed *zd = pkt->pkt.compressed;
@ -781,6 +787,7 @@ proc_compressed( CTX c, PACKET *pkt )
log_error("uncompressing failed: %s\n", g10_errstr(rc)); log_error("uncompressing failed: %s\n", g10_errstr(rc));
free_packet(pkt); free_packet(pkt);
c->last_was_session_key = 0; c->last_was_session_key = 0;
return rc;
} }
/**************** /****************
@ -1265,14 +1272,37 @@ proc_encryption_packets( void *anchor, IOBUF a )
} }
int static int
check_nesting (CTX c)
{
int level;
for (level = 0; c; c = c->anchor)
level++;
if (level > MAX_NESTING_DEPTH)
{
log_error ("input data with too deeply nested packets\n");
write_status_text (STATUS_UNEXPECTED, "1");
return G10ERR_UNEXPECTED;
}
return 0;
}
static int
do_proc_packets( CTX c, IOBUF a ) do_proc_packets( CTX c, IOBUF a )
{ {
PACKET *pkt = xmalloc( sizeof *pkt ); PACKET *pkt;
int rc=0; int rc = 0;
int any_data=0; int any_data = 0;
int newpkt; int newpkt;
rc = check_nesting (c);
if (rc)
return rc;
pkt = xmalloc( sizeof *pkt );
c->iobuf = a; c->iobuf = a;
init_packet(pkt); init_packet(pkt);
while( (rc=parse_packet(a, pkt)) != -1 ) { while( (rc=parse_packet(a, pkt)) != -1 ) {
@ -1293,7 +1323,7 @@ do_proc_packets( CTX c, IOBUF a )
case PKT_SYMKEY_ENC: proc_symkey_enc( c, pkt ); break; case PKT_SYMKEY_ENC: proc_symkey_enc( c, pkt ); break;
case PKT_ENCRYPTED: case PKT_ENCRYPTED:
case PKT_ENCRYPTED_MDC: proc_encrypted( c, pkt ); break; case PKT_ENCRYPTED_MDC: proc_encrypted( c, pkt ); break;
case PKT_COMPRESSED: proc_compressed( c, pkt ); break; case PKT_COMPRESSED: rc = proc_compressed( c, pkt ); break;
default: newpkt = 0; break; default: newpkt = 0; break;
} }
} }
@ -1311,7 +1341,7 @@ do_proc_packets( CTX c, IOBUF a )
goto leave; goto leave;
case PKT_SIGNATURE: newpkt = add_signature( c, pkt ); break; case PKT_SIGNATURE: newpkt = add_signature( c, pkt ); break;
case PKT_PLAINTEXT: proc_plaintext( c, pkt ); break; case PKT_PLAINTEXT: proc_plaintext( c, pkt ); break;
case PKT_COMPRESSED: proc_compressed( c, pkt ); break; case PKT_COMPRESSED: rc = proc_compressed( c, pkt ); break;
case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break; case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break;
case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break; case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break;
default: newpkt = 0; break; default: newpkt = 0; break;
@ -1331,7 +1361,7 @@ do_proc_packets( CTX c, IOBUF a )
case PKT_ENCRYPTED: case PKT_ENCRYPTED:
case PKT_ENCRYPTED_MDC: proc_encrypted( c, pkt ); break; case PKT_ENCRYPTED_MDC: proc_encrypted( c, pkt ); break;
case PKT_PLAINTEXT: proc_plaintext( c, pkt ); break; case PKT_PLAINTEXT: proc_plaintext( c, pkt ); break;
case PKT_COMPRESSED: proc_compressed( c, pkt ); break; case PKT_COMPRESSED: rc = proc_compressed( c, pkt ); break;
case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break; case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break;
case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break; case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break;
default: newpkt = 0; break; default: newpkt = 0; break;
@ -1356,13 +1386,17 @@ do_proc_packets( CTX c, IOBUF a )
case PKT_ENCRYPTED: case PKT_ENCRYPTED:
case PKT_ENCRYPTED_MDC: proc_encrypted( c, pkt ); break; case PKT_ENCRYPTED_MDC: proc_encrypted( c, pkt ); break;
case PKT_PLAINTEXT: proc_plaintext( c, pkt ); break; case PKT_PLAINTEXT: proc_plaintext( c, pkt ); break;
case PKT_COMPRESSED: proc_compressed( c, pkt ); break; case PKT_COMPRESSED: rc = proc_compressed( c, pkt ); break;
case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break; case PKT_ONEPASS_SIG: newpkt = add_onepass_sig( c, pkt ); break;
case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break; case PKT_GPG_CONTROL: newpkt = add_gpg_control(c, pkt); break;
case PKT_RING_TRUST: newpkt = add_ring_trust( c, pkt ); break; case PKT_RING_TRUST: newpkt = add_ring_trust( c, pkt ); break;
default: newpkt = 0; break; default: newpkt = 0; break;
} }
} }
if (rc)
goto leave;
/* This is a very ugly construct and frankly, I don't remember why /* This is a very ugly construct and frankly, I don't remember why
* I used it. Adding the MDC check here is a hack. * I used it. Adding the MDC check here is a hack.
* The right solution is to initiate another context for encrypted * The right solution is to initiate another context for encrypted