[PATCH GnuPG] gpg: Disallow compressed signatures and certificates
Demi Marie Obenour
demi at invisiblethingslab.com
Wed Jun 29 17:44:14 CEST 2022
Compressed packets have significant attack surface, both due to the
potential for 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.
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.
GnuPG-bug-id: T5993
Signed-off-by: Demi Marie Obenour <demi at invisiblethingslab.com>
---
g10/import.c | 18 ++----------------
g10/mainproc.c | 17 +++++++++++++++--
g10/packet.h | 2 ++
g10/parse-packet.c | 44 +++++++++++++++++++++++++++++++++++++++++++-
4 files changed, 62 insertions(+), 19 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..d85124abd7bb0067423835186f61a7f94b734aeb 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. */
@@ -1077,7 +1078,10 @@ proc_compressed (CTX c, PACKET *pkt)
/*printf("zip: compressed data packet\n");*/
if (c->sigs_only)
- rc = handle_compressed (c->ctrl, c, zd, proc_compressed_cb, c);
+ {
+ 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 +1600,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 +1612,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 +1655,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. */
;
--
Sincerely,
Demi Marie Obenour (she/her/hers)
Invisible Things Lab
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 833 bytes
Desc: not available
URL: <https://lists.gnupg.org/pipermail/gnupg-devel/attachments/20220629/46d9696a/attachment-0001.sig>
More information about the Gnupg-devel
mailing list