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 <w@1wt.eu>
(cherry picked from commit c54aabaf7c73e563a4cfdef760e2ae767612c601)
Signed-off-by: Christopher Faulet <cfaulet@haproxy.com>

diff --git a/include/haproxy/dns-t.h b/include/haproxy/dns-t.h
index 081332e..5f6212c 100644 (file)
--- 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 (file)
--- 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;