aboutsummaryrefslogtreecommitdiffstats
path: root/g10
diff options
context:
space:
mode:
Diffstat (limited to 'g10')
-rw-r--r--g10/ChangeLog15
-rw-r--r--g10/encr-data.c93
-rw-r--r--g10/openfile.c2
-rw-r--r--g10/parse-packet.c25
4 files changed, 104 insertions, 31 deletions
diff --git a/g10/ChangeLog b/g10/ChangeLog
index 193bf7b08..ebf49c329 100644
--- a/g10/ChangeLog
+++ b/g10/ChangeLog
@@ -1,3 +1,18 @@
+2006-12-07 Werner Koch <[email protected]>
+
+ * encr-data.c: Allocate DFX context on the heap and not on the
+ stack. Changes at several places. Fixes CVE-2006-6235.
+
+ * openfile.c (ask_outfile_name): Fixed buffer overflow occurring
+ if make_printable_string returns a longer string. Fixes bug 728.
+
+ * parse-packet.c (parse_user_id): Cap the user ID size at 2048
+ bytes. This prevents a memory allocation attack with a very large
+ user ID. A very large packet length could even cause the
+ allocation (a u32) to wrap around to a small number. Noted by
+ Evgeny Legerov on full-disclosure.
+ (parse_comment): Likewise.
+
2005-02-21 Werner Koch <[email protected]>
* seckey-cert.c (do_check): Detect card diversion protection.
diff --git a/g10/encr-data.c b/g10/encr-data.c
index fc76daf1d..70819e4ed 100644
--- a/g10/encr-data.c
+++ b/g10/encr-data.c
@@ -43,7 +43,27 @@ typedef struct {
char defer[20];
int defer_filled;
int eof_seen;
-} decode_filter_ctx_t;
+ int refcount;
+} *decode_filter_ctx_t;
+
+
+/* Helper to release the decode context. */
+static void
+release_dfx_context (decode_filter_ctx_t dfx)
+{
+ if (!dfx)
+ return;
+
+ assert (dfx->refcount);
+ if ( !--dfx->refcount )
+ {
+ cipher_close (dfx->cipher_hd);
+ dfx->cipher_hd = NULL;
+ md_close (dfx->mdc_hash);
+ dfx->mdc_hash = NULL;
+ m_free (dfx);
+ }
+}
/****************
@@ -59,7 +79,10 @@ decrypt_data( void *procctx, PKT_encrypted *ed, DEK *dek )
unsigned blocksize;
unsigned nprefix;
- memset( &dfx, 0, sizeof dfx );
+
+ dfx = m_alloc_clear (sizeof *dfx);
+ dfx->refcount = 1;
+
if( opt.verbose && !dek->algo_info_printed ) {
const char *s = cipher_algo_to_string( dek->algo );
if( s )
@@ -78,15 +101,15 @@ decrypt_data( void *procctx, PKT_encrypted *ed, DEK *dek )
BUG();
if( ed->mdc_method ) {
- dfx.mdc_hash = md_open( ed->mdc_method, 0 );
+ dfx->mdc_hash = md_open ( ed->mdc_method, 0 );
if ( DBG_HASHING )
- md_start_debug(dfx.mdc_hash, "checkmdc");
+ md_start_debug(dfx->mdc_hash, "checkmdc");
}
- dfx.cipher_hd = cipher_open( dek->algo,
- ed->mdc_method? CIPHER_MODE_CFB
- : CIPHER_MODE_AUTO_CFB, 1 );
+ dfx->cipher_hd = cipher_open ( dek->algo,
+ ed->mdc_method? CIPHER_MODE_CFB
+ : CIPHER_MODE_AUTO_CFB, 1 );
/* log_hexdump( "thekey", dek->key, dek->keylen );*/
- rc = cipher_setkey( dfx.cipher_hd, dek->key, dek->keylen );
+ rc = cipher_setkey( dfx->cipher_hd, dek->key, dek->keylen );
if( rc == G10ERR_WEAK_KEY )
log_info(_("WARNING: message was encrypted with "
"a weak key in the symmetric cipher.\n"));
@@ -99,7 +122,7 @@ decrypt_data( void *procctx, PKT_encrypted *ed, DEK *dek )
goto leave;
}
- cipher_setiv( dfx.cipher_hd, NULL, 0 );
+ cipher_setiv ( dfx->cipher_hd, NULL, 0 );
if( ed->len ) {
for(i=0; i < (nprefix+2) && ed->len; i++, ed->len-- ) {
@@ -116,8 +139,8 @@ decrypt_data( void *procctx, PKT_encrypted *ed, DEK *dek )
else
temp[i] = c;
}
- cipher_decrypt( dfx.cipher_hd, temp, temp, nprefix+2);
- cipher_sync( dfx.cipher_hd );
+ cipher_decrypt ( dfx->cipher_hd, temp, temp, nprefix+2);
+ cipher_sync ( dfx->cipher_hd );
p = temp;
/* log_hexdump( "prefix", temp, nprefix+2 ); */
if( dek->symmetric
@@ -127,34 +150,34 @@ decrypt_data( void *procctx, PKT_encrypted *ed, DEK *dek )
goto leave;
}
- if( dfx.mdc_hash )
- md_write( dfx.mdc_hash, temp, nprefix+2 );
+ if ( dfx->mdc_hash )
+ md_write ( dfx->mdc_hash, temp, nprefix+2 );
- if( ed->mdc_method )
- iobuf_push_filter( ed->buf, mdc_decode_filter, &dfx );
+ dfx->refcount++;
+ if ( ed->mdc_method )
+ iobuf_push_filter( ed->buf, mdc_decode_filter, dfx );
else
- iobuf_push_filter( ed->buf, decode_filter, &dfx );
+ iobuf_push_filter( ed->buf, decode_filter, dfx );
proc_packets( procctx, ed->buf );
ed->buf = NULL;
- if( ed->mdc_method && dfx.eof_seen == 2 )
+ if( ed->mdc_method && dfx->eof_seen == 2 )
rc = G10ERR_INVALID_PACKET;
else if( ed->mdc_method ) { /* check the mdc */
int datalen = md_digest_length( ed->mdc_method );
- cipher_decrypt( dfx.cipher_hd, dfx.defer, dfx.defer, 20);
- md_final( dfx.mdc_hash );
+ cipher_decrypt ( dfx->cipher_hd, dfx->defer, dfx->defer, 20);
+ md_final ( dfx->mdc_hash );
if( datalen != 20
- || memcmp(md_read( dfx.mdc_hash, 0 ), dfx.defer, datalen) )
+ || memcmp(md_read( dfx->mdc_hash, 0 ), dfx->defer, datalen) )
rc = G10ERR_BAD_SIGN;
- /*log_hexdump("MDC calculated:", md_read( dfx.mdc_hash, 0), datalen);*/
- /*log_hexdump("MDC message :", dfx.defer, 20);*/
+ /*log_hexdump("MDC calculated:",md_read( dfx->mdc_hash, 0), datalen);*/
+ /*log_hexdump("MDC message :", dfx->defer, 20);*/
}
leave:
- cipher_close(dfx.cipher_hd);
- md_close( dfx.mdc_hash );
+ release_dfx_context (dfx);
return rc;
}
@@ -165,7 +188,7 @@ static int
mdc_decode_filter( void *opaque, int control, IOBUF a,
byte *buf, size_t *ret_len)
{
- decode_filter_ctx_t *dfx = opaque;
+ decode_filter_ctx_t dfx = opaque;
size_t n, size = *ret_len;
int rc = 0;
int c;
@@ -220,8 +243,10 @@ mdc_decode_filter( void *opaque, int control, IOBUF a,
}
if( n ) {
- cipher_decrypt( dfx->cipher_hd, buf, buf, n);
- md_write( dfx->mdc_hash, buf, n );
+ if (dfx->cipher_hd)
+ cipher_decrypt( dfx->cipher_hd, buf, buf, n);
+ if (dfx->mdc_hash)
+ md_write( dfx->mdc_hash, buf, n );
}
else {
assert( dfx->eof_seen );
@@ -229,6 +254,9 @@ mdc_decode_filter( void *opaque, int control, IOBUF a,
}
*ret_len = n;
}
+ else if ( control == IOBUFCTRL_FREE ) {
+ release_dfx_context (dfx);
+ }
else if( control == IOBUFCTRL_DESC ) {
*(char**)buf = "mdc_decode_filter";
}
@@ -238,7 +266,7 @@ mdc_decode_filter( void *opaque, int control, IOBUF a,
static int
decode_filter( void *opaque, int control, IOBUF a, byte *buf, size_t *ret_len)
{
- decode_filter_ctx_t *fc = opaque;
+ decode_filter_ctx_t fc = opaque;
size_t n, size = *ret_len;
int rc = 0;
@@ -246,12 +274,17 @@ decode_filter( void *opaque, int control, IOBUF a, byte *buf, size_t *ret_len)
assert(a);
n = iobuf_read( a, buf, size );
if( n == -1 ) n = 0;
- if( n )
- cipher_decrypt( fc->cipher_hd, buf, buf, n);
+ if( n ) {
+ if (fc->cipher_hd)
+ cipher_decrypt( fc->cipher_hd, buf, buf, n);
+ }
else
rc = -1; /* eof */
*ret_len = n;
}
+ else if ( control == IOBUFCTRL_FREE ) {
+ release_dfx_context (fc);
+ }
else if( control == IOBUFCTRL_DESC ) {
*(char**)buf = "decode_filter";
}
diff --git a/g10/openfile.c b/g10/openfile.c
index c111303fa..702c7754d 100644
--- a/g10/openfile.c
+++ b/g10/openfile.c
@@ -140,8 +140,8 @@ ask_outfile_name( const char *name, size_t namelen )
s = _("Enter new filename");
- n = strlen(s) + namelen + 10;
defname = name && namelen? make_printable_string( name, namelen, 0): NULL;
+ n = strlen(s) + (defname?strlen (defname):0) + 10;
prompt = m_alloc(n);
if( defname )
sprintf(prompt, "%s [%s]: ", s, defname );
diff --git a/g10/parse-packet.c b/g10/parse-packet.c
index f9c4d16c2..21b3986df 100644
--- a/g10/parse-packet.c
+++ b/g10/parse-packet.c
@@ -1920,6 +1920,20 @@ parse_user_id( IOBUF inp, int pkttype, unsigned long pktlen, PACKET *packet )
{
byte *p;
+ /* Cap the size of a user ID at 2k: a value absurdly large enough
+ that there is no sane user ID string (which is printable text
+ as of RFC2440bis) that won't fit in it, but yet small enough to
+ avoid allocation problems. A large pktlen may not be
+ allocatable, and a very large pktlen could actually cause our
+ allocation to wrap around in xmalloc to a small number. */
+
+ if(pktlen>2048)
+ {
+ log_error("packet(%d) too large\n", pkttype);
+ skip_rest(inp, pktlen);
+ return G10ERR_INVALID_PACKET;
+ }
+
packet->pkt.user_id = m_alloc(sizeof *packet->pkt.user_id + pktlen);
packet->pkt.user_id->len = pktlen;
@@ -2014,6 +2028,17 @@ parse_comment( IOBUF inp, int pkttype, unsigned long pktlen, PACKET *packet )
{
byte *p;
+ /* Cap comment packet at a reasonable value to avoid an integer
+ 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)
+ {
+ log_error ("packet(%d) too large\n", pkttype);
+ skip_rest (inp, pktlen);
+ return G10ERR_INVALID_PACKET;
+ }
+
packet->pkt.comment = m_alloc(sizeof *packet->pkt.comment + pktlen - 1);
packet->pkt.comment->len = pktlen;
p = packet->pkt.comment->data;
generated by cgit v1.2.3 (git 2.25.1) at 2025-06-20 10:05:35 +0000