216 lines
7.9 KiB
Diff
216 lines
7.9 KiB
Diff
From 459b61fa21db755d6c879c3ef9ab85b3d1786c9f Mon Sep 17 00:00:00 2001
|
|
From: Demi Marie Obenour <demi () invisiblethingslab com>
|
|
Date: Fri, 27 May 2022 19:51:19 -0400
|
|
Subject: [PATCH GnuPG v3] Disallow compressed signatures and certificates
|
|
|
|
Compressed packets have significant attack surface, due to the potential
|
|
for both denial of service (zip bombs and the like) and for code
|
|
execution via memory corruption vulnerabilities in the decompressor.
|
|
Furthermore, I am not aware of any implementation that uses them in keys
|
|
or detached signatures. Therefore, disallow their use in such contexts
|
|
entirely. This includes signatures that are part of a cleartext-signed
|
|
message.
|
|
|
|
When parsing detached signatures, forbid any packet that is not a
|
|
signature or marker packet. When parsing keys, return an error when
|
|
encountering a compressed packet, instead of decompressing the packet.
|
|
|
|
Furthermore, certificates, keys, and signatures are not allowed to
|
|
contain partial-length or indeterminate-length packets. Reject those in
|
|
parse_packet, rather than activating the partial-length filter code.
|
|
This is not (yet) implemented for cleartext-signed messages, as these
|
|
messages are internally represented as inline-signed messages.
|
|
|
|
GnuPG-bug-id: T5993
|
|
Signed-off-by: Demi Marie Obenour <demiobenour () gmail com>
|
|
---
|
|
g10/import.c | 18 ++----------------
|
|
g10/mainproc.c | 24 +++++++++++++++++++++---
|
|
g10/packet.h | 2 ++
|
|
g10/parse-packet.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
|
|
4 files changed, 68 insertions(+), 20 deletions(-)
|
|
|
|
diff --git a/g10/import.c b/g10/import.c
|
|
index bb0bf67934a8316130cde182cd43d56353e0171d..a8136351f6f7dae8c65634ed8e1c242d323e2009 100644
|
|
--- a/g10/import.c
|
|
+++ b/g10/import.c
|
|
@@ -1042,22 +1042,8 @@ read_block( IOBUF a, unsigned int options,
|
|
switch (pkt->pkttype)
|
|
{
|
|
case PKT_COMPRESSED:
|
|
- if (check_compress_algo (pkt->pkt.compressed->algorithm))
|
|
- {
|
|
- rc = GPG_ERR_COMPR_ALGO;
|
|
- goto ready;
|
|
- }
|
|
- else
|
|
- {
|
|
- compress_filter_context_t *cfx = xmalloc_clear( sizeof *cfx );
|
|
- pkt->pkt.compressed->buf = NULL;
|
|
- if (push_compress_filter2 (a, cfx,
|
|
- pkt->pkt.compressed->algorithm, 1))
|
|
- xfree (cfx); /* e.g. in case of compression_algo NONE. */
|
|
- }
|
|
- free_packet (pkt, &parsectx);
|
|
- init_packet(pkt);
|
|
- break;
|
|
+ rc = GPG_ERR_UNEXPECTED;
|
|
+ goto ready;
|
|
|
|
case PKT_RING_TRUST:
|
|
/* Skip those packets unless we are in restore mode. */
|
|
diff --git a/g10/mainproc.c b/g10/mainproc.c
|
|
index af11877aa257e46662c42b6ff573ee01c3ad1547..3629fc921b742afd131e8d8e2664b201095990f0 100644
|
|
--- a/g10/mainproc.c
|
|
+++ b/g10/mainproc.c
|
|
@@ -152,6 +152,7 @@ add_onepass_sig (CTX c, PACKET *pkt)
|
|
{
|
|
kbnode_t node;
|
|
|
|
+ log_assert(!(c->sigs_only && c->signed_data.used));
|
|
if (c->list) /* Add another packet. */
|
|
add_kbnode (c->list, new_kbnode (pkt));
|
|
else /* Insert the first one. */
|
|
@@ -1076,8 +1077,16 @@ proc_compressed (CTX c, PACKET *pkt)
|
|
int rc;
|
|
|
|
/*printf("zip: compressed data packet\n");*/
|
|
- if (c->sigs_only)
|
|
- rc = handle_compressed (c->ctrl, c, zd, proc_compressed_cb, c);
|
|
+ if ( literals_seen )
|
|
+ {
|
|
+ log_error ("Compressed packet follows literal data packet\n");
|
|
+ rc = GPG_ERR_UNEXPECTED;
|
|
+ }
|
|
+ else if ( c->sigs_only )
|
|
+ {
|
|
+ log_assert(!c->signed_data.used);
|
|
+ rc = handle_compressed (c->ctrl, c, zd, proc_compressed_cb, c);
|
|
+ }
|
|
else if( c->encrypt_only )
|
|
rc = handle_compressed (c->ctrl, c, zd, proc_encrypt_cb, c);
|
|
else
|
|
@@ -1596,6 +1605,7 @@ do_proc_packets (CTX c, iobuf_t a)
|
|
c->iobuf = a;
|
|
init_packet(pkt);
|
|
init_parse_packet (&parsectx, a);
|
|
+ parsectx.sigs_only = c->sigs_only && c->signed_data.used;
|
|
while ((rc=parse_packet (&parsectx, pkt)) != -1)
|
|
{
|
|
any_data = 1;
|
|
@@ -1607,6 +1617,12 @@ do_proc_packets (CTX c, iobuf_t a)
|
|
if (gpg_err_code (rc) == GPG_ERR_INV_PACKET
|
|
&& opt.list_packets == 0)
|
|
break;
|
|
+
|
|
+ if (gpg_err_code (rc) == GPG_ERR_UNEXPECTED)
|
|
+ {
|
|
+ write_status_text( STATUS_UNEXPECTED, "0" );
|
|
+ goto leave;
|
|
+ }
|
|
continue;
|
|
}
|
|
newpkt = -1;
|
|
@@ -1644,7 +1660,9 @@ do_proc_packets (CTX c, iobuf_t a)
|
|
case PKT_COMPRESSED: rc = proc_compressed (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;
|
|
- default: newpkt = 0; break;
|
|
+ default:
|
|
+ log_assert(!c->signed_data.used);
|
|
+ newpkt = 0; break;
|
|
}
|
|
}
|
|
else if (c->encrypt_only)
|
|
diff --git a/g10/packet.h b/g10/packet.h
|
|
index 5a14015a16c872fe7b0b15468598daf7a05ffc02..82dfe786b46051491e7015e64441678140defa9e 100644
|
|
--- a/g10/packet.h
|
|
+++ b/g10/packet.h
|
|
@@ -657,6 +657,7 @@ struct parse_packet_ctx_s
|
|
int free_last_pkt; /* Indicates that LAST_PKT must be freed. */
|
|
int skip_meta; /* Skip ring trust packets. */
|
|
unsigned int n_parsed_packets; /* Number of parsed packets. */
|
|
+ int sigs_only; /* Only accept detached signature packets */
|
|
};
|
|
typedef struct parse_packet_ctx_s *parse_packet_ctx_t;
|
|
|
|
@@ -667,6 +668,7 @@ typedef struct parse_packet_ctx_s *parse_packet_ctx_t;
|
|
(a)->free_last_pkt = 0; \
|
|
(a)->skip_meta = 0; \
|
|
(a)->n_parsed_packets = 0; \
|
|
+ (a)->sigs_only = 0; \
|
|
} while (0)
|
|
|
|
#define deinit_parse_packet(a) do { \
|
|
diff --git a/g10/parse-packet.c b/g10/parse-packet.c
|
|
index cea1f7ebc5daec3863ae963c1ab25500f86796fe..dca66ff427ea6778e536782ec6bda83584877342 100644
|
|
--- a/g10/parse-packet.c
|
|
+++ b/g10/parse-packet.c
|
|
@@ -738,6 +738,20 @@ parse (parse_packet_ctx_t ctx, PACKET *pkt, int onlykeypkts, off_t * retpos,
|
|
case PKT_ENCRYPTED_MDC:
|
|
case PKT_ENCRYPTED_AEAD:
|
|
case PKT_COMPRESSED:
|
|
+ if (ctx->sigs_only)
|
|
+ {
|
|
+ log_error (_("partial length packet of type %d in detached"
|
|
+ " signature\n"), pkttype);
|
|
+ rc = gpg_error (GPG_ERR_UNEXPECTED);
|
|
+ goto leave;
|
|
+ }
|
|
+ if (onlykeypkts)
|
|
+ {
|
|
+ log_error (_("partial length packet of type %d in keyring\n"),
|
|
+ pkttype);
|
|
+ rc = gpg_error (GPG_ERR_UNEXPECTED);
|
|
+ goto leave;
|
|
+ }
|
|
iobuf_set_partial_body_length_mode (inp, c & 0xff);
|
|
pktlen = 0; /* To indicate partial length. */
|
|
partial = 1;
|
|
@@ -775,6 +789,20 @@ parse (parse_packet_ctx_t ctx, PACKET *pkt, int onlykeypkts, off_t * retpos,
|
|
rc = gpg_error (GPG_ERR_INV_PACKET);
|
|
goto leave;
|
|
}
|
|
+ else if (ctx->sigs_only)
|
|
+ {
|
|
+ log_error (_("indeterminate length packet of type %d in detached"
|
|
+ " signature\n"), pkttype);
|
|
+ rc = gpg_error (GPG_ERR_UNEXPECTED);
|
|
+ goto leave;
|
|
+ }
|
|
+ else if (onlykeypkts)
|
|
+ {
|
|
+ log_error (_("indeterminate length packet of type %d in"
|
|
+ " keyring\n"), pkttype);
|
|
+ rc = gpg_error (GPG_ERR_UNEXPECTED);
|
|
+ goto leave;
|
|
+ }
|
|
}
|
|
else
|
|
{
|
|
@@ -828,7 +856,21 @@ parse (parse_packet_ctx_t ctx, PACKET *pkt, int onlykeypkts, off_t * retpos,
|
|
goto leave;
|
|
}
|
|
|
|
- if (with_uid && pkttype == PKT_USER_ID)
|
|
+ if (ctx->sigs_only)
|
|
+ switch (pkttype)
|
|
+ {
|
|
+ case PKT_SIGNATURE:
|
|
+ case PKT_MARKER:
|
|
+ break;
|
|
+ default:
|
|
+ log_error(_("Packet type %d not allowed in detached signature\n"),
|
|
+ pkttype);
|
|
+ iobuf_skip_rest (inp, pktlen, partial);
|
|
+ *skip = 1;
|
|
+ rc = GPG_ERR_UNEXPECTED;
|
|
+ goto leave;
|
|
+ }
|
|
+ else if (with_uid && pkttype == PKT_USER_ID)
|
|
/* If ONLYKEYPKTS is set to 2, then we never skip user id packets,
|
|
even if DO_SKIP is set. */
|
|
;
|
|
--
|
|
2.36.1
|
|
|