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
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
|
From f6b1f8e7d642f82d830a71b78699a4349e0158e1 Mon Sep 17 00:00:00 2001
From: Jeremy Harris <jgh146exb@wizmail.org>
Date: Tue, 10 Oct 2023 23:03:28 +0100
Subject: [PATCH 3/3] Harden dnsdb against crafted DNS responses. Bug 3033
(cherry picked from commit 8787c8994f07c23c3664d76926e02f07314d699d)
---
doc/ChangeLog | 3 ++
src/dns.c | 11 +++---
src/lookups/dnsdb.c | 78 +++++++++++++++++++++++++++--------------
3 files changed, 59 insertions(+), 33 deletions(-)
diff --git a/src/dns.c b/src/dns.c
index 7d7ee0c04..8dc3695a1 100644
--- a/src/dns.c
+++ b/src/dns.c
@@ -300,15 +300,15 @@ return string_from_gstring(g);
/* Increment the aptr in dnss, checking against dnsa length.
Return: TRUE for a bad result
*/
static BOOL
dnss_inc_aptr(const dns_answer * dnsa, dns_scan * dnss, unsigned delta)
{
-return (dnss->aptr += delta) >= dnsa->answer + dnsa->answerlen;
+return (dnss->aptr += delta) > dnsa->answer + dnsa->answerlen;
}
/*************************************************
* Get next DNS record from answer block *
*************************************************/
/* Call this with reset == RESET_ANSWERS to scan the answer block, reset ==
@@ -384,15 +384,15 @@ if (reset != RESET_NEXT)
namelen = dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen,
dnss->aptr, (DN_EXPAND_ARG4_TYPE) &dnss->srr.name, DNS_MAXNAME);
if (namelen < 0) goto null_return;
/* skip name, type, class & TTL */
TRACE trace = "A-hdr";
if (dnss_inc_aptr(dnsa, dnss, namelen+8)) goto null_return;
GETSHORT(dnss->srr.size, dnss->aptr); /* size of data portion */
- /* skip over it */
+ /* skip over it, checking for a bogus size */
TRACE trace = "A-skip";
if (dnss_inc_aptr(dnsa, dnss, dnss->srr.size)) goto null_return;
}
dnss->rrcount = reset == RESET_AUTHORITY
? ntohs(h->nscount) : ntohs(h->arcount);
TRACE debug_printf("%s: reset (%s rrcount %d)\n", __FUNCTION__,
reset == RESET_AUTHORITY ? "NS" : "AR", dnss->rrcount);
@@ -424,18 +424,17 @@ if (dnss_inc_aptr(dnsa, dnss, namelen)) goto null_return;
GETSHORT(dnss->srr.type, dnss->aptr); /* Record type */
TRACE trace = "R-class";
if (dnss_inc_aptr(dnsa, dnss, 2)) goto null_return; /* Don't want class */
GETLONG(dnss->srr.ttl, dnss->aptr); /* TTL */
GETSHORT(dnss->srr.size, dnss->aptr); /* Size of data portion */
dnss->srr.data = dnss->aptr; /* The record's data follows */
-/* Unchecked increment ok here since no further access on this iteration;
-will be checked on next at "R-name". */
-
-dnss->aptr += dnss->srr.size; /* Advance to next RR */
+/* skip over it, checking for a bogus size */
+if (dnss_inc_aptr(dnsa, dnss, dnss->srr.size))
+ goto null_return;
/* Return a pointer to the dns_record structure within the dns_answer. This is
for convenience so that the scans can use nice-looking for loops. */
TRACE debug_printf("%s: return %s\n", __FUNCTION__, dns_text_type(dnss->srr.type));
return &dnss->srr;
diff --git a/src/lookups/dnsdb.c b/src/lookups/dnsdb.c
index 355be1b5d..020dc9a52 100644
--- a/src/lookups/dnsdb.c
+++ b/src/lookups/dnsdb.c
@@ -394,80 +394,101 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
/* Other kinds of record just have one piece of data each, but there may be
several of them, of course. */
if (yield->ptr) yield = string_catn(yield, outsep, 1);
if (type == T_TXT || type == T_SPF)
{
- if (outsep2 == NULL) /* output only the first item of data */
- yield = string_catn(yield, US (rr->data+1), (rr->data)[0]);
+ if (!outsep2) /* output only the first item of data */
+ {
+ uschar n = (rr->data)[0];
+ /* size byte + data bytes must not excced the RRs length */
+ if (n + 1 <= rr->size)
+ yield = string_catn(yield, US (rr->data+1), n);
+ }
else
{
/* output all items */
int data_offset = 0;
while (data_offset < rr->size)
{
- uschar chunk_len = (rr->data)[data_offset++];
- if (outsep2[0] != '\0' && data_offset != 1)
+ uschar chunk_len = (rr->data)[data_offset];
+ int remain = rr->size - data_offset;
+
+ /* Apparently there are resolvers that do not check RRs before passing
+ them on, and glibc fails to do so. So every application must...
+ Check for chunk len exceeding RR */
+
+ if (chunk_len > remain)
+ chunk_len = remain;
+
+ if (*outsep2 && data_offset != 0)
yield = string_catn(yield, outsep2, 1);
- yield = string_catn(yield, US ((rr->data)+data_offset), chunk_len);
+ yield = string_catn(yield, US ((rr->data) + ++data_offset), --chunk_len);
data_offset += chunk_len;
}
}
}
else if (type == T_TLSA)
- {
- uint8_t usage, selector, matching_type;
- uint16_t payload_length;
- uschar s[MAX_TLSA_EXPANDED_SIZE];
- uschar * sp = s;
- uschar * p = US rr->data;
+ if (rr->size < 3)
+ continue;
+ else
+ {
+ uint8_t usage, selector, matching_type;
+ uint16_t payload_length;
+ uschar s[MAX_TLSA_EXPANDED_SIZE];
+ uschar * sp = s;
+ uschar * p = US rr->data;
+
+ usage = *p++;
+ selector = *p++;
+ matching_type = *p++;
+ /* What's left after removing the first 3 bytes above */
+ payload_length = rr->size - 3;
+ sp += sprintf(CS s, "%d%c%d%c%d%c", usage, *outsep2,
+ selector, *outsep2, matching_type, *outsep2);
+ /* Now append the cert/identifier, one hex char at a time */
+ while (payload_length-- > 0 && sp-s < (MAX_TLSA_EXPANDED_SIZE - 4))
+ sp += sprintf(CS sp, "%02x", *p++);
- usage = *p++;
- selector = *p++;
- matching_type = *p++;
- /* What's left after removing the first 3 bytes above */
- payload_length = rr->size - 3;
- sp += sprintf(CS s, "%d%c%d%c%d%c", usage, *outsep2,
- selector, *outsep2, matching_type, *outsep2);
- /* Now append the cert/identifier, one hex char at a time */
- while (payload_length-- > 0 && sp-s < (MAX_TLSA_EXPANDED_SIZE - 4))
- sp += sprintf(CS sp, "%02x", *p++);
-
- yield = string_cat(yield, s);
- }
+ yield = string_cat(yield, s);
+ }
else /* T_CNAME, T_CSA, T_MX, T_MXH, T_NS, T_PTR, T_SOA, T_SRV */
{
int priority, weight, port;
uschar s[264];
uschar * p = US rr->data;
switch (type)
{
case T_MXH:
+ if (rr->size < sizeof(u_int16_t)) continue;
/* mxh ignores the priority number and includes only the hostnames */
GETSHORT(priority, p);
break;
case T_MX:
+ if (rr->size < sizeof(u_int16_t)) continue;
GETSHORT(priority, p);
sprintf(CS s, "%d%c", priority, *outsep2);
yield = string_cat(yield, s);
break;
case T_SRV:
+ if (rr->size < 3*sizeof(u_int16_t)) continue;
GETSHORT(priority, p);
GETSHORT(weight, p);
GETSHORT(port, p);
sprintf(CS s, "%d%c%d%c%d%c", priority, *outsep2,
weight, *outsep2, port, *outsep2);
yield = string_cat(yield, s);
break;
case T_CSA:
+ if (rr->size < 3*sizeof(u_int16_t)) continue;
/* See acl_verify_csa() for more comments about CSA. */
GETSHORT(priority, p);
GETSHORT(weight, p);
GETSHORT(port, p);
if (priority != 1) continue; /* CSA version must be 1 */
@@ -510,15 +531,15 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
"domain=%s", dns_text_type(type), domain);
break;
}
else yield = string_cat(yield, s);
if (type == T_SOA && outsep2 != NULL)
{
- unsigned long serial, refresh, retry, expire, minimum;
+ unsigned long serial = 0, refresh = 0, retry = 0, expire = 0, minimum = 0;
p += rc;
yield = string_catn(yield, outsep2, 1);
rc = dn_expand(dnsa->answer, dnsa->answer + dnsa->answerlen, p,
(DN_EXPAND_ARG4_TYPE)s, sizeof(s));
if (rc < 0)
@@ -526,16 +547,19 @@ while ((domain = string_nextinlist(&keystring, &sep, NULL, 0)))
log_write(0, LOG_MAIN, "responsible-mailbox truncated: type=%s "
"domain=%s", dns_text_type(type), domain);
break;
}
else yield = string_cat(yield, s);
p += rc;
- GETLONG(serial, p); GETLONG(refresh, p);
- GETLONG(retry, p); GETLONG(expire, p); GETLONG(minimum, p);
+ if (rr->size >= p - rr->data - 5*sizeof(u_int32_t))
+ {
+ GETLONG(serial, p); GETLONG(refresh, p);
+ GETLONG(retry, p); GETLONG(expire, p); GETLONG(minimum, p);
+ }
sprintf(CS s, "%c%lu%c%lu%c%lu%c%lu%c%lu",
*outsep2, serial, *outsep2, refresh,
*outsep2, retry, *outsep2, expire, *outsep2, minimum);
yield = string_cat(yield, s);
}
}
} /* Loop for list of returned records */
--
2.42.0
|
