From c89e11c8fa64641edddf9a055745d825ae3fab9d Mon Sep 17 00:00:00 2001 From: me-no-dev Date: Mon, 13 Mar 2017 11:56:50 +0200 Subject: [PATCH] address security issues with mDNS --- components/mdns/mdns.c | 43 +++++++++++++++++++++++++++--------------- 1 file changed, 28 insertions(+), 15 deletions(-) diff --git a/components/mdns/mdns.c b/components/mdns/mdns.c index d636134c7d..d0fc69c4cc 100644 --- a/components/mdns/mdns.c +++ b/components/mdns/mdns.c @@ -491,7 +491,7 @@ static const uint8_t * _mdns_read_fqdn(const uint8_t * packet, const uint8_t * s && (strcmp(buf, MDNS_DEFAULT_DOMAIN) != 0) && (strcmp(buf, "ip6") != 0) && (strcmp(buf, "in-addr") != 0)) { - sprintf((char*)name, "%s.%s", name->host, buf); + snprintf((char*)name, MDNS_NAME_BUF_LEN, "%s.%s", name->host, buf); } else if (strcmp(buf, MDNS_SUB_STR) == 0) { name->sub = 1; } else { @@ -499,7 +499,7 @@ static const uint8_t * _mdns_read_fqdn(const uint8_t * packet, const uint8_t * s } } else { size_t address = (((uint16_t)len & 0x3F) << 8) | start[index++]; - if ((packet + address) > start) { + if ((packet + address) >= start) { //reference address can not be after where we are return NULL; } @@ -1182,13 +1182,13 @@ static mdns_service_t * _mdns_create_service(const char * service, const char * s->txt = NULL; s->port = port; - s->service = strdup(service); + s->service = strndup(service, MDNS_NAME_BUF_LEN - 1); if (!s->service) { free(s); return NULL; } - s->proto = strdup(proto); + s->proto = strndup(proto, MDNS_NAME_BUF_LEN - 1); if (!s->proto) { free((char *)s->service); free(s); @@ -1341,7 +1341,7 @@ static void _mdns_parse_packet(mdns_server_t * server, const uint8_t * data, siz (strcmp(name->proto, server->search.proto) != 0)) { continue;//not searching for service or wrong service/proto } - sprintf(answer->instance, "%s", name->host); + strlcpy(answer->instance, name->host, MDNS_NAME_BUF_LEN); } else if (type == MDNS_TYPE_SRV) { if (server->search.host[0] || (strcmp(name->service, server->search.service) != 0) || @@ -1353,7 +1353,7 @@ static void _mdns_parse_packet(mdns_server_t * server, const uint8_t * data, siz continue;//instance name is not the same as the one in the PTR record } } else { - sprintf(answer->instance, "%s", name->host); + strlcpy(answer->instance, name->host, MDNS_NAME_BUF_LEN); } //parse record value if (!_mdns_parse_fqdn(data, data_ptr + MDNS_SRV_FQDN_OFFSET, name)) { @@ -1367,15 +1367,19 @@ static void _mdns_parse_packet(mdns_server_t * server, const uint8_t * data, siz if (answer->host[0]) { if (strcmp(answer->host, name->host) != 0) { answer->addr = 0; - sprintf(answer->host, "%s", name->host); + strlcpy(answer->host, name->host, MDNS_NAME_BUF_LEN); } } else { - sprintf(answer->host, "%s", name->host); + strlcpy(answer->host, name->host, MDNS_NAME_BUF_LEN); } } else if (type == MDNS_TYPE_TXT) { uint16_t i=0,b=0, y; while(i < data_len) { uint8_t partLen = data_ptr[i++]; + //check if partLen will fit in the buffer + if (partLen > (MDNS_TXT_MAX_LEN - b - 1)) { + break; + } for(y=0; ytxt[b++] = d; @@ -1391,7 +1395,7 @@ static void _mdns_parse_packet(mdns_server_t * server, const uint8_t * data, siz continue;//wrong host } } else if (!answer->ptr) { - sprintf(answer->host, "%s", name->host); + strlcpy(answer->host, name->host, MDNS_NAME_BUF_LEN); } else if (strcmp(answer->host, name->host) != 0) { continue;//wrong host } @@ -1402,7 +1406,7 @@ static void _mdns_parse_packet(mdns_server_t * server, const uint8_t * data, siz continue;//wrong host } } else if (!answer->ptr) { - sprintf(answer->host, "%s", name->host); + strlcpy(answer->host, name->host, MDNS_NAME_BUF_LEN); } else if (strcmp(answer->host, name->host) != 0) { continue;//wrong host } @@ -1534,6 +1538,9 @@ esp_err_t mdns_set_hostname(mdns_server_t * server, const char * hostname) if (!server) { return ESP_ERR_INVALID_ARG; } + if (strlen(hostname) > (MDNS_NAME_BUF_LEN - 1)) { + return ESP_ERR_INVALID_ARG; + } MDNS_MUTEX_LOCK(); free((char*)server->hostname); server->hostname = (char *)malloc(strlen(hostname)+1); @@ -1541,7 +1548,7 @@ esp_err_t mdns_set_hostname(mdns_server_t * server, const char * hostname) MDNS_MUTEX_UNLOCK(); return ESP_ERR_NO_MEM; } - sprintf((char *)server->hostname, "%s", hostname); + strlcpy((char *)server->hostname, hostname, MDNS_NAME_BUF_LEN); MDNS_MUTEX_UNLOCK(); return ERR_OK; } @@ -1551,6 +1558,9 @@ esp_err_t mdns_set_instance(mdns_server_t * server, const char * instance) if (!server) { return ESP_ERR_INVALID_ARG; } + if (strlen(instance) > (MDNS_NAME_BUF_LEN - 1)) { + return ESP_ERR_INVALID_ARG; + } MDNS_MUTEX_LOCK(); free((char*)server->instance); server->instance = (char *)malloc(strlen(instance)+1); @@ -1558,7 +1568,7 @@ esp_err_t mdns_set_instance(mdns_server_t * server, const char * instance) MDNS_MUTEX_UNLOCK(); return ESP_ERR_NO_MEM; } - sprintf((char *)server->instance, "%s", instance); + strlcpy((char *)server->instance, instance, MDNS_NAME_BUF_LEN); MDNS_MUTEX_UNLOCK(); return ERR_OK; } @@ -1655,6 +1665,9 @@ esp_err_t mdns_service_instance_set(mdns_server_t * server, const char * service if (!server || !server->services || !service || !proto) { return ESP_ERR_INVALID_ARG; } + if (strlen(instance) > (MDNS_NAME_BUF_LEN - 1)) { + return ESP_ERR_INVALID_ARG; + } mdns_srv_item_t * s = _mdns_get_service_item(server, service, proto); if (!s) { return ESP_ERR_NOT_FOUND; @@ -1741,10 +1754,10 @@ uint32_t mdns_query(mdns_server_t * server, const char * service, const char * p mdns_result_free(server); if (proto) { server->search.host[0] = 0; - snprintf(server->search.service, MDNS_NAME_MAX_LEN, "%s", service); - snprintf(server->search.proto, MDNS_NAME_MAX_LEN, "%s", proto); + strlcpy(server->search.service, service, MDNS_NAME_BUF_LEN); + strlcpy(server->search.proto, proto, MDNS_NAME_BUF_LEN); } else { - snprintf(server->search.host, MDNS_NAME_MAX_LEN, "%s", service); + strlcpy(server->search.host, service, MDNS_NAME_BUF_LEN); server->search.service[0] = 0; server->search.proto[0] = 0; qtype = MDNS_TYPE_A; -- 2.40.0