From 909a0d7ed3fcd424f0c61b344edd5b3e9d44ceb0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Petr=20Men=C5=A1=C3=ADk?= Date: Tue, 5 Apr 2022 04:07:30 +0200 Subject: [PATCH] Attempt to fix insufficient extract_name buffer (#7293) * Attempt to fix insufficient extract_name buffer Some fuzzing failures happen because extract_name always uses in real dnsmasq code daemon->namebuff of size at least MAXDNAME. Provide long enough data also to fuzzed functions. * Add myself as an interested party Being dnsmasq maintainer in Fedora project and RHEL, I am interested in new found failures. Especially when security related. * Allocate full dhcp packet buffer fuzz_dhcp can fail in clear packet. But that fails, because clear packet always cleans whole buffer of maximal DHCP packet. But fuzzer allocates less. Fix fuzzer to allocate similar memory as dhcp_common_init() function of real dnsmasq. --- projects/dnsmasq/fuzz_dhcp.c | 12 +++--------- projects/dnsmasq/fuzz_header.h | 31 ++++++++++++++++++++++--------- projects/dnsmasq/fuzz_rfc1035.c | 6 +++--- projects/dnsmasq/fuzz_util.c | 4 ++-- projects/dnsmasq/project.yaml | 1 + 5 files changed, 31 insertions(+), 23 deletions(-) diff --git a/projects/dnsmasq/fuzz_dhcp.c b/projects/dnsmasq/fuzz_dhcp.c index 4ba61ca3b..d17e618ea 100644 --- a/projects/dnsmasq/fuzz_dhcp.c +++ b/projects/dnsmasq/fuzz_dhcp.c @@ -24,14 +24,14 @@ void FuzzDhcp(const uint8_t **data2, size_t *size2) { struct iovec *dhpa = malloc(sizeof(struct iovec)); if (dhpa == NULL) return; - char *content = malloc(300); + char *content = malloc(sizeof(struct dhcp_packet)); if (content == NULL) { free(dhpa); return; } dhpa->iov_base = content; - dhpa->iov_len = 300; + dhpa->iov_len = sizeof(struct dhcp_packet); daemon->dhcp_packet = *dhpa; @@ -42,13 +42,7 @@ void FuzzDhcp(const uint8_t **data2, size_t *size2) { // dnsmasq may change the iov_base if the buffer needs expansion. // Do not free in that case, only free if the buffer stays that same. - if (daemon->dhcp_packet.iov_base == content) { - free(content); - } - else{ - free(daemon->dhcp_packet.iov_base); - } - + free(daemon->dhcp_packet.iov_base); free(dhpa); } diff --git a/projects/dnsmasq/fuzz_header.h b/projects/dnsmasq/fuzz_header.h index a865b9c17..cccee38aa 100644 --- a/projects/dnsmasq/fuzz_header.h +++ b/projects/dnsmasq/fuzz_header.h @@ -38,21 +38,25 @@ void gb_cleanup() { } } -char *get_null_terminated(const uint8_t **data, size_t *size) { -#define STR_SIZE 75 - if (*size < STR_SIZE || (int)*size < 0) { +char *get_len_null_terminated(const uint8_t **data, size_t *size, size_t to_get) { + if (*size < to_get || (int)*size < 0) { return NULL; } - char *new_s = malloc(STR_SIZE + 1); - memcpy(new_s, *data, STR_SIZE); - new_s[STR_SIZE] = '\0'; + char *new_s = malloc(to_get + 1); + memcpy(new_s, *data, to_get); + new_s[to_get] = '\0'; - *data = *data+STR_SIZE; - *size -= STR_SIZE; + *data = *data+to_get; + *size -= to_get; return new_s; } +char *get_null_terminated(const uint8_t **data, size_t *size) { +#define STR_SIZE 75 + return get_len_null_terminated(data, size, STR_SIZE); +} + char *gb_get_random_data(const uint8_t **data, size_t *size, size_t to_get) { if (*size < to_get || (int)*size < 0) { return NULL; @@ -79,6 +83,15 @@ char *gb_get_null_terminated(const uint8_t **data, size_t *size) { return nstr; } +char *gb_get_len_null_terminated(const uint8_t **data, size_t *size, size_t to_get) { + + char *nstr = get_len_null_terminated(data, size, to_get); + if (nstr != NULL) { + pointer_arr[pointer_idx++] = (void*)nstr; + } + return nstr; +} + char *gb_alloc_data(size_t len) { char *ptr = calloc(1, len); pointer_arr[pointer_idx++] = (void*)ptr; @@ -235,7 +248,7 @@ int init_daemon(const uint8_t **data2, size_t *size2) { daemon->min_cache_ttl = get_int(&data, &size); // daemon->namebuff. - char *daemon_namebuff = gb_get_null_terminated(&data, &size); + char *daemon_namebuff = gb_get_len_null_terminated(&data, &size, MAXDNAME); daemon->namebuff = daemon_namebuff; // daemon->naptr diff --git a/projects/dnsmasq/fuzz_rfc1035.c b/projects/dnsmasq/fuzz_rfc1035.c index fb563f804..c9546fbaa 100644 --- a/projects/dnsmasq/fuzz_rfc1035.c +++ b/projects/dnsmasq/fuzz_rfc1035.c @@ -20,7 +20,7 @@ void FuzzExtractTheAddress(const uint8_t **data2, size_t *size2) { size_t size = *size2; char *new_name = NULL; - new_name = get_null_terminated(&data, &size); + new_name = get_len_null_terminated(&data, &size, MAXDNAME); pointer_arr[pointer_idx++] = (void*)new_name; int is_sign = get_int(&data, &size); @@ -110,7 +110,7 @@ void FuzzExtractRequest(const uint8_t **data2, size_t *size2) { size_t size = *size2; char *new_name = NULL; - new_name = get_null_terminated(&data, &size); + new_name = get_len_null_terminated(&data, &size, MAXDNAME); if (new_name == NULL) { return ; @@ -187,7 +187,7 @@ void FuzzCheckForBogusWildcard(const uint8_t **data2, size_t *size2) { const uint8_t *data = *data2; size_t size = *size2; - char *nname = gb_get_null_terminated(&data, &size); + char *nname = gb_get_len_null_terminated(&data, &size, MAXDNAME); if (nname == NULL) { return; } diff --git a/projects/dnsmasq/fuzz_util.c b/projects/dnsmasq/fuzz_util.c index 7ab723ccd..6edc6d8b7 100644 --- a/projects/dnsmasq/fuzz_util.c +++ b/projects/dnsmasq/fuzz_util.c @@ -18,8 +18,8 @@ int LLVMFuzzerTestOneInput(const uint8_t *data, size_t size) { int succ = init_daemon(&data, &size); if (succ == 0) { - char *t1 = gb_get_null_terminated(&data, &size); - char *t2 = gb_get_null_terminated(&data, &size); + char *t1 = gb_get_len_null_terminated(&data, &size, MAXDNAME); + char *t2 = gb_get_len_null_terminated(&data, &size, MAXDNAME); if (t1 != NULL && t2 != NULL) { // Util logic diff --git a/projects/dnsmasq/project.yaml b/projects/dnsmasq/project.yaml index 51d7e1e07..3717c9eb5 100644 --- a/projects/dnsmasq/project.yaml +++ b/projects/dnsmasq/project.yaml @@ -4,3 +4,4 @@ primary_contact: "simon@thekelleys.org.uk" main_repo: "git://thekelleys.org.uk/dnsmasq.git" auto_ccs: - "david@adalogics.com" + - "pemensik@redhat.com"