From 43f5d16760c0fe0d818d0bb9fb0f26e3782fdc80 Mon Sep 17 00:00:00 2001 From: millert Date: Wed, 15 Mar 2023 22:12:00 +0000 Subject: [PATCH] Fix a number of out of bound reads in DNS response parsing. Originally from djm@. OK deraadt@ florian@ bluhm@ --- lib/libc/asr/asr_utils.c | 6 ++-- lib/libc/asr/getrrsetbyname_async.c | 51 +++++++++++++++++++++++------ lib/libc/net/res_comp.c | 8 ++++- 3 files changed, 51 insertions(+), 14 deletions(-) diff --git a/lib/libc/asr/asr_utils.c b/lib/libc/asr/asr_utils.c index 1b856761e82..9ae9bfa863b 100644 --- a/lib/libc/asr/asr_utils.c +++ b/lib/libc/asr/asr_utils.c @@ -1,4 +1,4 @@ -/* $OpenBSD: asr_utils.c,v 1.20 2022/12/27 17:10:06 jmc Exp $ */ +/* $OpenBSD: asr_utils.c,v 1.21 2023/03/15 22:12:00 millert Exp $ */ /* * Copyright (c) 2009-2012 Eric Faurot * @@ -123,7 +123,7 @@ dname_expand(const unsigned char *data, size_t len, size_t offset, for (; (n = data[offset]); ) { if ((n & 0xc0) == 0xc0) { - if (offset + 2 > len) + if (offset + 1 >= len) return (-1); ptr = 256 * (n & ~0xc0) + data[offset + 1]; if (ptr >= start) @@ -133,7 +133,7 @@ dname_expand(const unsigned char *data, size_t len, size_t offset, offset = start = ptr; continue; } - if (offset + n + 1 > len) + if (offset + n + 1 >= len) return (-1); if (dname_check_label(data + offset + 1, n) == -1) diff --git a/lib/libc/asr/getrrsetbyname_async.c b/lib/libc/asr/getrrsetbyname_async.c index 28e86d2fc1e..43e6cfec606 100644 --- a/lib/libc/asr/getrrsetbyname_async.c +++ b/lib/libc/asr/getrrsetbyname_async.c @@ -1,4 +1,4 @@ -/* $OpenBSD: getrrsetbyname_async.c,v 1.12 2022/12/27 17:10:06 jmc Exp $ */ +/* $OpenBSD: getrrsetbyname_async.c,v 1.13 2023/03/15 22:12:00 millert Exp $ */ /* * Copyright (c) 2012 Eric Faurot * @@ -170,7 +170,7 @@ getrrsetbyname_async_run(struct asr_query *as, struct asr_result *ar) /* The rest of this file is taken from the original implementation. */ -/* $OpenBSD: getrrsetbyname_async.c,v 1.12 2022/12/27 17:10:06 jmc Exp $ */ +/* $OpenBSD: getrrsetbyname_async.c,v 1.13 2023/03/15 22:12:00 millert Exp $ */ /* * Copyright (c) 2001 Jakob Schlyter. All rights reserved. @@ -365,6 +365,9 @@ parse_dns_response(const u_char *answer, int size) struct dns_response *resp; const u_char *cp; + if (size <= HFIXEDSZ) + return (NULL); + /* allocate memory for the response */ resp = calloc(1, sizeof(*resp)); if (resp == NULL) @@ -431,14 +434,22 @@ parse_dns_qsection(const u_char *answer, int size, const u_char **cp, int count) int i, length; char name[MAXDNAME]; - for (i = 1, head = NULL, prev = NULL; i <= count; i++, prev = curr) { +#define NEED(need) \ + do { \ + if (*cp + need > answer + size) \ + goto fail; \ + } while (0) - /* allocate and initialize struct */ - curr = calloc(1, sizeof(struct dns_query)); - if (curr == NULL) { + for (i = 1, head = NULL, prev = NULL; i <= count; i++, prev = curr) { + if (*cp >= answer + size) { + fail: free_dns_query(head); return (NULL); } + /* allocate and initialize struct */ + curr = calloc(1, sizeof(struct dns_query)); + if (curr == NULL) + goto fail; if (head == NULL) head = curr; if (prev != NULL) @@ -456,16 +467,20 @@ parse_dns_qsection(const u_char *answer, int size, const u_char **cp, int count) free_dns_query(head); return (NULL); } + NEED(length); *cp += length; /* type */ + NEED(INT16SZ); curr->type = _getshort(*cp); *cp += INT16SZ; /* class */ + NEED(INT16SZ); curr->class = _getshort(*cp); *cp += INT16SZ; } +#undef NEED return (head); } @@ -478,14 +493,23 @@ parse_dns_rrsection(const u_char *answer, int size, const u_char **cp, int i, length; char name[MAXDNAME]; - for (i = 1, head = NULL, prev = NULL; i <= count; i++, prev = curr) { +#define NEED(need) \ + do { \ + if (*cp + need > answer + size) \ + goto fail; \ + } while (0) - /* allocate and initialize struct */ - curr = calloc(1, sizeof(struct dns_rr)); - if (curr == NULL) { + for (i = 1, head = NULL, prev = NULL; i <= count; i++, prev = curr) { + if (*cp >= answer + size) { + fail: free_dns_rr(head); return (NULL); } + + /* allocate and initialize struct */ + curr = calloc(1, sizeof(struct dns_rr)); + if (curr == NULL) + goto fail; if (head == NULL) head = curr; if (prev != NULL) @@ -503,25 +527,31 @@ parse_dns_rrsection(const u_char *answer, int size, const u_char **cp, free_dns_rr(head); return (NULL); } + NEED(length); *cp += length; /* type */ + NEED(INT16SZ); curr->type = _getshort(*cp); *cp += INT16SZ; /* class */ + NEED(INT16SZ); curr->class = _getshort(*cp); *cp += INT16SZ; /* ttl */ + NEED(INT32SZ); curr->ttl = _getlong(*cp); *cp += INT32SZ; /* rdata size */ + NEED(INT16SZ); curr->size = _getshort(*cp); *cp += INT16SZ; /* rdata itself */ + NEED(curr->size); curr->rdata = malloc(curr->size); if (curr->rdata == NULL) { free_dns_rr(head); @@ -530,6 +560,7 @@ parse_dns_rrsection(const u_char *answer, int size, const u_char **cp, memcpy(curr->rdata, *cp, curr->size); *cp += curr->size; } +#undef NEED return (head); } diff --git a/lib/libc/net/res_comp.c b/lib/libc/net/res_comp.c index 7ccd44ad8d5..ce9f92ae771 100644 --- a/lib/libc/net/res_comp.c +++ b/lib/libc/net/res_comp.c @@ -1,4 +1,4 @@ -/* $OpenBSD: res_comp.c,v 1.22 2022/12/27 17:10:06 jmc Exp $ */ +/* $OpenBSD: res_comp.c,v 1.23 2023/03/15 22:12:00 millert Exp $ */ /* * ++Copyright++ 1985, 1993 @@ -82,6 +82,9 @@ dn_expand(const u_char *msg, const u_char *eomorig, const u_char *comp_dn, char *eom; int len = -1, checked = 0; + if (comp_dn < msg || comp_dn >= eomorig) + return (-1); + dn = exp_dn; cp = comp_dn; if (length > HOST_NAME_MAX) @@ -91,6 +94,9 @@ dn_expand(const u_char *msg, const u_char *eomorig, const u_char *comp_dn, * fetch next label in domain name */ while ((n = *cp++)) { + if (cp >= eomorig) /* out of range */ + return (-1); + /* * Check for indirection */ -- 2.20.1