From 342b785ad5ccbcadf7d60b881c349c21b2d7d6d4 Mon Sep 17 00:00:00 2001 From: Willy Tarreau Date: Thu, 14 Oct 2021 22:30:38 +0200 Subject: [PATCH] BUG/MEDIUM: resolvers: use correct storage for the target address The struct resolv_answer_item contains an address field of type "sockaddr" which is only 16 bytes long, but which is used to store either IPv4 or IPv6. Fortunately, the contents only overlap with the "target" field that follows it and that is large enough to absorb the extra bytes needed to store AAAA records. But this is dangerous as just moving fields around could result in memory corruption. The fix uses a union and removes the casts that were used to hide the problem. Older versions need to be checked and possibly fixed. This needs to be backported anyway. (cherry picked from commit b4ca0195a9591614bfcebe9b7fe80c1ce9f94d8d) Signed-off-by: Willy Tarreau (cherry picked from commit c54aabaf7c73e563a4cfdef760e2ae767612c601) Signed-off-by: Christopher Faulet --- include/haproxy/dns-t.h | 5 ++++- src/dns.c | 52 ++++++++++++++++++++++------------------------- 2 files changed, 28 insertions(+), 29 deletions(-) diff --git a/include/haproxy/dns-t.h b/include/haproxy/dns-t.h index 081332e..5f6212c 100644 --- a/include/haproxy/dns-t.h +++ b/include/haproxy/dns-t.h @@ -145,7 +145,10 @@ struct dns_answer_item { uint16_t weight; /* SRV type weight */ uint16_t port; /* SRV type port */ uint16_t data_len; /* number of bytes in target below */ - struct sockaddr address; /* IPv4 or IPv6, network format */ + union { + struct sockaddr_in in4; /* IPv4 address for RTYPE_A */ + struct sockaddr_in6 in6; /* IPv6 address for RTYPE_AAAA */ + } address; char target[DNS_MAX_NAME_SIZE+1]; /* Response data: SRV or CNAME type target */ unsigned int last_seen; /* When was the answer was last seen */ struct dns_answer_item *ar_item; /* pointer to a RRset from the additional section, if exists */ diff --git a/src/dns.c b/src/dns.c index 4090ef5..874afeb 100644 --- a/src/dns.c +++ b/src/dns.c @@ -821,10 +821,10 @@ srv_found: switch (item->ar_item->type) { case DNS_RTYPE_A: - update_server_addr(srv, &(((struct sockaddr_in*)&item->ar_item->address)->sin_addr), AF_INET, "DNS additional record"); + update_server_addr(srv, &item->ar_item->address.in4.sin_addr, AF_INET, "DNS additional record"); break; case DNS_RTYPE_AAAA: - update_server_addr(srv, &(((struct sockaddr_in6*)&item->ar_item->address)->sin6_addr), AF_INET6, "DNS additional record"); + update_server_addr(srv, &item->ar_item->address.in6.sin6_addr, AF_INET6, "DNS additional record"); break; } @@ -1132,9 +1132,8 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, if (dns_answer_record->data_len != 4) goto invalid_resp; - dns_answer_record->address.sa_family = AF_INET; - memcpy(&(((struct sockaddr_in *)&dns_answer_record->address)->sin_addr), - reader, dns_answer_record->data_len); + dns_answer_record->address.in4.sin_family = AF_INET; + memcpy(&dns_answer_record->address.in4.sin_addr, reader, dns_answer_record->data_len); break; case DNS_RTYPE_CNAME: @@ -1197,9 +1196,8 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, if (dns_answer_record->data_len != 16) goto invalid_resp; - dns_answer_record->address.sa_family = AF_INET6; - memcpy(&(((struct sockaddr_in6 *)&dns_answer_record->address)->sin6_addr), - reader, dns_answer_record->data_len); + dns_answer_record->address.in6.sin6_family = AF_INET6; + memcpy(&dns_answer_record->address.in6.sin6_addr, reader, dns_answer_record->data_len); break; } /* switch (record type) */ @@ -1222,16 +1220,16 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, switch(tmp_record->type) { case DNS_RTYPE_A: - if (!memcmp(&((struct sockaddr_in *)&dns_answer_record->address)->sin_addr, - &((struct sockaddr_in *)&tmp_record->address)->sin_addr, - sizeof(in_addr_t))) + if (!memcmp(&dns_answer_record->address.in4.sin_addr, + &tmp_record->address.in4.sin_addr, + sizeof(dns_answer_record->address.in4.sin_addr))) found = 1; break; case DNS_RTYPE_AAAA: - if (!memcmp(&((struct sockaddr_in6 *)&dns_answer_record->address)->sin6_addr, - &((struct sockaddr_in6 *)&tmp_record->address)->sin6_addr, - sizeof(struct in6_addr))) + if (!memcmp(&dns_answer_record->address.in6.sin6_addr, + &tmp_record->address.in6.sin6_addr, + sizeof(dns_answer_record->address.in6.sin6_addr))) found = 1; break; @@ -1368,9 +1366,8 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, if (dns_answer_record->data_len != 4) goto invalid_resp; - dns_answer_record->address.sa_family = AF_INET; - memcpy(&(((struct sockaddr_in *)&dns_answer_record->address)->sin_addr), - reader, dns_answer_record->data_len); + dns_answer_record->address.in4.sin_family = AF_INET; + memcpy(&dns_answer_record->address.in4.sin_addr, reader, dns_answer_record->data_len); break; case DNS_RTYPE_AAAA: @@ -1378,9 +1375,8 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, if (dns_answer_record->data_len != 16) goto invalid_resp; - dns_answer_record->address.sa_family = AF_INET6; - memcpy(&(((struct sockaddr_in6 *)&dns_answer_record->address)->sin6_addr), - reader, dns_answer_record->data_len); + dns_answer_record->address.in6.sin6_family = AF_INET6; + memcpy(&dns_answer_record->address.in6.sin6_addr, reader, dns_answer_record->data_len); break; default: @@ -1414,16 +1410,16 @@ static int dns_validate_dns_response(unsigned char *resp, unsigned char *bufend, switch(ar_item->type) { case DNS_RTYPE_A: - if (!memcmp(&((struct sockaddr_in *)&dns_answer_record->address)->sin_addr, - &((struct sockaddr_in *)&ar_item->address)->sin_addr, - sizeof(in_addr_t))) + if (!memcmp(&dns_answer_record->address.in4.sin_addr, + &ar_item->address.in4.sin_addr, + sizeof(dns_answer_record->address.in4.sin_addr))) found = 1; break; case DNS_RTYPE_AAAA: - if (!memcmp(&((struct sockaddr_in6 *)&dns_answer_record->address)->sin6_addr, - &((struct sockaddr_in6 *)&ar_item->address)->sin6_addr, - sizeof(struct in6_addr))) + if (!memcmp(&dns_answer_record->address.in6.sin6_addr, + &ar_item->address.in6.sin6_addr, + sizeof(dns_answer_record->address.in6.sin6_addr))) found = 1; break; @@ -1531,12 +1527,12 @@ int dns_get_ip_from_response(struct dns_response_packet *dns_p, unsigned char ip_type; if (record->type == DNS_RTYPE_A) { - ip = &(((struct sockaddr_in *)&record->address)->sin_addr); ip_type = AF_INET; + ip = &record->address.in4.sin_addr; } else if (record->type == DNS_RTYPE_AAAA) { ip_type = AF_INET6; - ip = &(((struct sockaddr_in6 *)&record->address)->sin6_addr); + ip = &record->address.in6.sin6_addr; } else continue; -- 1.7.10.4