1
0

0102-Fix-remote-buffer-overflow-CERT-VU-434904.patch 10 KB

123456789101112131415161718192021222324252627282930313233343536373839404142434445464748495051525354555657585960616263646566676869707172737475767778798081828384858687888990919293949596979899100101102103104105106107108109110111112113114115116117118119120121122123124125126127128129130131132133134135136137138139140141142143144145146147148149150151152153154155156157158159160161162163164165166167168169170171172173174175176177178179180181182183184185186187188189190191192193194195196197198199200201202203204205206207208209210211212213214215216217218219220221222223224225226227228229230231232233234235236237238239240241242243244245246247248249250251252253254255256257258259260261262263264265266267268269270271272273274275276277278279280281282283284285286287288289290291292293294295296297298299300301302303304305306307308309310311312313314315316317318319320321322323324325326327328329330331332333334335336337338339340341342343344345346347348349350351352353354355356357358359360361362363364365366367368369370371372373374375
  1. From 4e96a4be685c9e4445f6ee79ad0b36b9119b502a Mon Sep 17 00:00:00 2001
  2. From: Simon Kelley <simon@thekelleys.org.uk>
  3. Date: Wed, 11 Nov 2020 23:25:04 +0000
  4. Subject: Fix remote buffer overflow CERT VU#434904
  5. The problem is in the sort_rrset() function and allows a remote
  6. attacker to overwrite memory. Any dnsmasq instance with DNSSEC
  7. enabled is vulnerable.
  8. ---
  9. CHANGELOG | 7 +-
  10. src/dnssec.c | 273 ++++++++++++++++++++++++++++-----------------------
  11. 2 files changed, 158 insertions(+), 122 deletions(-)
  12. --- a/CHANGELOG
  13. +++ b/CHANGELOG
  14. @@ -1,3 +1,9 @@
  15. + Fix a remote buffer overflow problem in the DNSSEC code. Any
  16. + dnsmasq with DNSSEC compiled in and enabled is vulnerable to this,
  17. + referenced by CERT VU#434904.
  18. +
  19. +
  20. +>>>>>>> Fix remote buffer overflow CERT VU#434904
  21. version 2.81
  22. Impove cache behaviour for TCP connections. For ease of
  23. implementaion, dnsmasq has always forked a new process to handle
  24. --- a/src/dnssec.c
  25. +++ b/src/dnssec.c
  26. @@ -222,138 +222,147 @@ static int check_date_range(u32 date_sta
  27. && serial_compare_32(curtime, date_end) == SERIAL_LT;
  28. }
  29. -/* Return bytes of canonicalised rdata, when the return value is zero, the remaining
  30. - data, pointed to by *p, should be used raw. */
  31. -static int get_rdata(struct dns_header *header, size_t plen, unsigned char *end, char *buff, int bufflen,
  32. - unsigned char **p, u16 **desc)
  33. +/* Return bytes of canonicalised rrdata one by one.
  34. + Init state->ip with the RR, and state->end with the end of same.
  35. + Init state->op to NULL.
  36. + Init state->desc to RR descriptor.
  37. + Init state->buff with a MAXDNAME * 2 buffer.
  38. +
  39. + After each call which returns 1, state->op points to the next byte of data.
  40. + On returning 0, the end has been reached.
  41. +*/
  42. +struct rdata_state {
  43. + u16 *desc;
  44. + size_t c;
  45. + unsigned char *end, *ip, *op;
  46. + char *buff;
  47. +};
  48. +
  49. +static int get_rdata(struct dns_header *header, size_t plen, struct rdata_state *state)
  50. {
  51. - int d = **desc;
  52. + int d;
  53. - /* No more data needs mangling */
  54. - if (d == (u16)-1)
  55. + if (state->op && state->c != 1)
  56. {
  57. - /* If there's more data than we have space for, just return what fits,
  58. - we'll get called again for more chunks */
  59. - if (end - *p > bufflen)
  60. - {
  61. - memcpy(buff, *p, bufflen);
  62. - *p += bufflen;
  63. - return bufflen;
  64. - }
  65. -
  66. - return 0;
  67. + state->op++;
  68. + state->c--;
  69. + return 1;
  70. }
  71. -
  72. - (*desc)++;
  73. -
  74. - if (d == 0 && extract_name(header, plen, p, buff, 1, 0))
  75. - /* domain-name, canonicalise */
  76. - return to_wire(buff);
  77. - else
  78. - {
  79. - /* plain data preceding a domain-name, don't run off the end of the data */
  80. - if ((end - *p) < d)
  81. - d = end - *p;
  82. +
  83. + while (1)
  84. + {
  85. + d = *(state->desc);
  86. - if (d != 0)
  87. + if (d == (u16)-1)
  88. {
  89. - memcpy(buff, *p, d);
  90. - *p += d;
  91. + /* all the bytes to the end. */
  92. + if ((state->c = state->end - state->ip) != 0)
  93. + {
  94. + state->op = state->ip;
  95. + state->ip = state->end;;
  96. + }
  97. + else
  98. + return 0;
  99. + }
  100. + else
  101. + {
  102. + state->desc++;
  103. +
  104. + if (d == (u16)0)
  105. + {
  106. + /* domain-name, canonicalise */
  107. + int len;
  108. +
  109. + if (!extract_name(header, plen, &state->ip, state->buff, 1, 0) ||
  110. + (len = to_wire(state->buff)) == 0)
  111. + continue;
  112. +
  113. + state->c = len;
  114. + state->op = (unsigned char *)state->buff;
  115. + }
  116. + else
  117. + {
  118. + /* plain data preceding a domain-name, don't run off the end of the data */
  119. + if ((state->end - state->ip) < d)
  120. + d = state->end - state->ip;
  121. +
  122. + if (d == 0)
  123. + continue;
  124. +
  125. + state->op = state->ip;
  126. + state->c = d;
  127. + state->ip += d;
  128. + }
  129. }
  130. - return d;
  131. + return 1;
  132. }
  133. }
  134. -/* Bubble sort the RRset into the canonical order.
  135. - Note that the byte-streams from two RRs may get unsynced: consider
  136. - RRs which have two domain-names at the start and then other data.
  137. - The domain-names may have different lengths in each RR, but sort equal
  138. -
  139. - ------------
  140. - |abcde|fghi|
  141. - ------------
  142. - |abcd|efghi|
  143. - ------------
  144. -
  145. - leaving the following bytes as deciding the order. Hence the nasty left1 and left2 variables.
  146. -*/
  147. +/* Bubble sort the RRset into the canonical order. */
  148. static int sort_rrset(struct dns_header *header, size_t plen, u16 *rr_desc, int rrsetidx,
  149. unsigned char **rrset, char *buff1, char *buff2)
  150. {
  151. - int swap, quit, i, j;
  152. + int swap, i, j;
  153. do
  154. {
  155. for (swap = 0, i = 0; i < rrsetidx-1; i++)
  156. {
  157. - int rdlen1, rdlen2, left1, left2, len1, len2, len, rc;
  158. - u16 *dp1, *dp2;
  159. - unsigned char *end1, *end2;
  160. + int rdlen1, rdlen2;
  161. + struct rdata_state state1, state2;
  162. +
  163. /* Note that these have been determined to be OK previously,
  164. so we don't need to check for NULL return here. */
  165. - unsigned char *p1 = skip_name(rrset[i], header, plen, 10);
  166. - unsigned char *p2 = skip_name(rrset[i+1], header, plen, 10);
  167. -
  168. - p1 += 8; /* skip class, type, ttl */
  169. - GETSHORT(rdlen1, p1);
  170. - end1 = p1 + rdlen1;
  171. -
  172. - p2 += 8; /* skip class, type, ttl */
  173. - GETSHORT(rdlen2, p2);
  174. - end2 = p2 + rdlen2;
  175. -
  176. - dp1 = dp2 = rr_desc;
  177. -
  178. - for (quit = 0, left1 = 0, left2 = 0, len1 = 0, len2 = 0; !quit;)
  179. + state1.ip = skip_name(rrset[i], header, plen, 10);
  180. + state2.ip = skip_name(rrset[i+1], header, plen, 10);
  181. + state1.op = state2.op = NULL;
  182. + state1.buff = buff1;
  183. + state2.buff = buff2;
  184. + state1.desc = state2.desc = rr_desc;
  185. +
  186. + state1.ip += 8; /* skip class, type, ttl */
  187. + GETSHORT(rdlen1, state1.ip);
  188. + if (!CHECK_LEN(header, state1.ip, plen, rdlen1))
  189. + return rrsetidx; /* short packet */
  190. + state1.end = state1.ip + rdlen1;
  191. +
  192. + state2.ip += 8; /* skip class, type, ttl */
  193. + GETSHORT(rdlen2, state2.ip);
  194. + if (!CHECK_LEN(header, state2.ip, plen, rdlen2))
  195. + return rrsetidx; /* short packet */
  196. + state2.end = state2.ip + rdlen2;
  197. +
  198. + while (1)
  199. {
  200. - if (left1 != 0)
  201. - memmove(buff1, buff1 + len1 - left1, left1);
  202. -
  203. - if ((len1 = get_rdata(header, plen, end1, buff1 + left1, (MAXDNAME * 2) - left1, &p1, &dp1)) == 0)
  204. - {
  205. - quit = 1;
  206. - len1 = end1 - p1;
  207. - memcpy(buff1 + left1, p1, len1);
  208. - }
  209. - len1 += left1;
  210. -
  211. - if (left2 != 0)
  212. - memmove(buff2, buff2 + len2 - left2, left2);
  213. -
  214. - if ((len2 = get_rdata(header, plen, end2, buff2 + left2, (MAXDNAME *2) - left2, &p2, &dp2)) == 0)
  215. - {
  216. - quit = 1;
  217. - len2 = end2 - p2;
  218. - memcpy(buff2 + left2, p2, len2);
  219. - }
  220. - len2 += left2;
  221. -
  222. - if (len1 > len2)
  223. - left1 = len1 - len2, left2 = 0, len = len2;
  224. - else
  225. - left2 = len2 - len1, left1 = 0, len = len1;
  226. + int ok1, ok2;
  227. - rc = (len == 0) ? 0 : memcmp(buff1, buff2, len);
  228. -
  229. - if (rc > 0 || (rc == 0 && quit && len1 > len2))
  230. - {
  231. - unsigned char *tmp = rrset[i+1];
  232. - rrset[i+1] = rrset[i];
  233. - rrset[i] = tmp;
  234. - swap = quit = 1;
  235. - }
  236. - else if (rc == 0 && quit && len1 == len2)
  237. + ok1 = get_rdata(header, plen, &state1);
  238. + ok2 = get_rdata(header, plen, &state2);
  239. +
  240. + if (!ok1 && !ok2)
  241. {
  242. /* Two RRs are equal, remove one copy. RFC 4034, para 6.3 */
  243. for (j = i+1; j < rrsetidx-1; j++)
  244. rrset[j] = rrset[j+1];
  245. rrsetidx--;
  246. i--;
  247. + break;
  248. + }
  249. + else if (ok1 && (!ok2 || *state1.op > *state2.op))
  250. + {
  251. + unsigned char *tmp = rrset[i+1];
  252. + rrset[i+1] = rrset[i];
  253. + rrset[i] = tmp;
  254. + swap = 1;
  255. + break;
  256. }
  257. - else if (rc < 0)
  258. - quit = 1;
  259. + else if (ok2 && (!ok1 || *state2.op > *state1.op))
  260. + break;
  261. +
  262. + /* arrive here when bytes are equal, go round the loop again
  263. + and compare the next ones. */
  264. }
  265. }
  266. } while (swap);
  267. @@ -549,15 +558,18 @@ static int validate_rrset(time_t now, st
  268. wire_len = to_wire(keyname);
  269. hash->update(ctx, (unsigned int)wire_len, (unsigned char*)keyname);
  270. from_wire(keyname);
  271. +
  272. +#define RRBUFLEN 300 /* Most RRs are smaller than this. */
  273. for (i = 0; i < rrsetidx; ++i)
  274. {
  275. - int seg;
  276. - unsigned char *end, *cp;
  277. - u16 len, *dp;
  278. + int j;
  279. + struct rdata_state state;
  280. + u16 len;
  281. + unsigned char rrbuf[RRBUFLEN];
  282. p = rrset[i];
  283. -
  284. +
  285. if (!extract_name(header, plen, &p, name, 1, 10))
  286. return STAT_BOGUS;
  287. @@ -566,12 +578,11 @@ static int validate_rrset(time_t now, st
  288. /* if more labels than in RRsig name, hash *.<no labels in rrsig labels field> 4035 5.3.2 */
  289. if (labels < name_labels)
  290. {
  291. - int k;
  292. - for (k = name_labels - labels; k != 0; k--)
  293. + for (j = name_labels - labels; j != 0; j--)
  294. {
  295. while (*name_start != '.' && *name_start != 0)
  296. name_start++;
  297. - if (k != 1 && *name_start == '.')
  298. + if (j != 1 && *name_start == '.')
  299. name_start++;
  300. }
  301. @@ -592,24 +603,44 @@ static int validate_rrset(time_t now, st
  302. if (!CHECK_LEN(header, p, plen, rdlen))
  303. return STAT_BOGUS;
  304. - end = p + rdlen;
  305. -
  306. - /* canonicalise rdata and calculate length of same, use name buffer as workspace.
  307. - Note that name buffer is twice MAXDNAME long in DNSSEC mode. */
  308. - cp = p;
  309. - dp = rr_desc;
  310. - for (len = 0; (seg = get_rdata(header, plen, end, name, MAXDNAME * 2, &cp, &dp)) != 0; len += seg);
  311. - len += end - cp;
  312. - len = htons(len);
  313. + /* canonicalise rdata and calculate length of same, use
  314. + name buffer as workspace for get_rdata. */
  315. + state.ip = p;
  316. + state.op = NULL;
  317. + state.desc = rr_desc;
  318. + state.buff = name;
  319. + state.end = p + rdlen;
  320. +
  321. + for (j = 0; get_rdata(header, plen, &state); j++)
  322. + if (j < RRBUFLEN)
  323. + rrbuf[j] = *state.op;
  324. +
  325. + len = htons((u16)j);
  326. hash->update(ctx, 2, (unsigned char *)&len);
  327. +
  328. + /* If the RR is shorter than RRBUFLEN (most of them, in practice)
  329. + then we can just digest it now. If it exceeds RRBUFLEN we have to
  330. + go back to the start and do it in chunks. */
  331. + if (j >= RRBUFLEN)
  332. + {
  333. + state.ip = p;
  334. + state.op = NULL;
  335. + state.desc = rr_desc;
  336. +
  337. + for (j = 0; get_rdata(header, plen, &state); j++)
  338. + {
  339. + rrbuf[j] = *state.op;
  340. +
  341. + if (j == RRBUFLEN - 1)
  342. + {
  343. + hash->update(ctx, RRBUFLEN, rrbuf);
  344. + j = -1;
  345. + }
  346. + }
  347. + }
  348. - /* Now canonicalise again and digest. */
  349. - cp = p;
  350. - dp = rr_desc;
  351. - while ((seg = get_rdata(header, plen, end, name, MAXDNAME * 2, &cp, &dp)))
  352. - hash->update(ctx, seg, (unsigned char *)name);
  353. - if (cp != end)
  354. - hash->update(ctx, end - cp, cp);
  355. + if (j != 0)
  356. + hash->update(ctx, j, rrbuf);
  357. }
  358. hash->digest(ctx, hash->digest_size, digest);