1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
|
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
|
