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.
diff --git a/include/haproxy/resolvers-t.h b/include/haproxy/resolvers-t.h
index 463e24b..251b9f1 100644
--- a/include/haproxy/resolvers-t.h
+++ b/include/haproxy/resolvers-t.h
@@ -111,7 +111,10 @@
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 resolv_answer_item *ar_item; /* pointer to a RRset from the additional section, if exists */
diff --git a/src/resolvers.c b/src/resolvers.c
index ec50d19..1144597 100644
--- a/src/resolvers.c
+++ b/src/resolvers.c
@@ -761,10 +761,10 @@
switch (item->ar_item->type) {
case DNS_RTYPE_A:
- srv_update_addr(srv, &(((struct sockaddr_in*)&item->ar_item->address)->sin_addr), AF_INET, "DNS additional record");
+ srv_update_addr(srv, &item->ar_item->address.in4.sin_addr, AF_INET, "DNS additional record");
break;
case DNS_RTYPE_AAAA:
- srv_update_addr(srv, &(((struct sockaddr_in6*)&item->ar_item->address)->sin6_addr), AF_INET6, "DNS additional record");
+ srv_update_addr(srv, &item->ar_item->address.in6.sin6_addr, AF_INET6, "DNS additional record");
break;
}
@@ -1069,9 +1069,8 @@
if (answer_record->data_len != 4)
goto invalid_resp;
- answer_record->address.sa_family = AF_INET;
- memcpy(&(((struct sockaddr_in *)&answer_record->address)->sin_addr),
- reader, answer_record->data_len);
+ answer_record->address.in4.sin_family = AF_INET;
+ memcpy(&answer_record->address.in4.sin_addr, reader, answer_record->data_len);
break;
case DNS_RTYPE_CNAME:
@@ -1134,9 +1133,8 @@
if (answer_record->data_len != 16)
goto invalid_resp;
- answer_record->address.sa_family = AF_INET6;
- memcpy(&(((struct sockaddr_in6 *)&answer_record->address)->sin6_addr),
- reader, answer_record->data_len);
+ answer_record->address.in6.sin6_family = AF_INET6;
+ memcpy(&answer_record->address.in6.sin6_addr, reader, answer_record->data_len);
break;
} /* switch (record type) */
@@ -1159,16 +1157,16 @@
switch(tmp_record->type) {
case DNS_RTYPE_A:
- if (!memcmp(&((struct sockaddr_in *)&answer_record->address)->sin_addr,
- &((struct sockaddr_in *)&tmp_record->address)->sin_addr,
- sizeof(in_addr_t)))
+ if (!memcmp(&answer_record->address.in4.sin_addr,
+ &tmp_record->address.in4.sin_addr,
+ sizeof(answer_record->address.in4.sin_addr)))
found = 1;
break;
case DNS_RTYPE_AAAA:
- if (!memcmp(&((struct sockaddr_in6 *)&answer_record->address)->sin6_addr,
- &((struct sockaddr_in6 *)&tmp_record->address)->sin6_addr,
- sizeof(struct in6_addr)))
+ if (!memcmp(&answer_record->address.in6.sin6_addr,
+ &tmp_record->address.in6.sin6_addr,
+ sizeof(answer_record->address.in6.sin6_addr)))
found = 1;
break;
@@ -1305,9 +1303,8 @@
if (answer_record->data_len != 4)
goto invalid_resp;
- answer_record->address.sa_family = AF_INET;
- memcpy(&(((struct sockaddr_in *)&answer_record->address)->sin_addr),
- reader, answer_record->data_len);
+ answer_record->address.in4.sin_family = AF_INET;
+ memcpy(&answer_record->address.in4.sin_addr, reader, answer_record->data_len);
break;
case DNS_RTYPE_AAAA:
@@ -1315,9 +1312,8 @@
if (answer_record->data_len != 16)
goto invalid_resp;
- answer_record->address.sa_family = AF_INET6;
- memcpy(&(((struct sockaddr_in6 *)&answer_record->address)->sin6_addr),
- reader, answer_record->data_len);
+ answer_record->address.in6.sin6_family = AF_INET6;
+ memcpy(&answer_record->address.in6.sin6_addr, reader, answer_record->data_len);
break;
default:
@@ -1351,16 +1347,16 @@
switch(ar_item->type) {
case DNS_RTYPE_A:
- if (!memcmp(&((struct sockaddr_in *)&answer_record->address)->sin_addr,
- &((struct sockaddr_in *)&ar_item->address)->sin_addr,
- sizeof(in_addr_t)))
+ if (!memcmp(&answer_record->address.in4.sin_addr,
+ &ar_item->address.in4.sin_addr,
+ sizeof(answer_record->address.in4.sin_addr)))
found = 1;
break;
case DNS_RTYPE_AAAA:
- if (!memcmp(&((struct sockaddr_in6 *)&answer_record->address)->sin6_addr,
- &((struct sockaddr_in6 *)&ar_item->address)->sin6_addr,
- sizeof(struct in6_addr)))
+ if (!memcmp(&answer_record->address.in6.sin6_addr,
+ &ar_item->address.in6.sin6_addr,
+ sizeof(answer_record->address.in6.sin6_addr)))
found = 1;
break;
@@ -1469,12 +1465,12 @@
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;