Skip to content

Commit b203126

Browse files
committed
Replace the signed-int (... & 0xFF) << 24 reassembly in dns_callback's A-record
path with the safe memcpy+ee32 get_be32 helper (hoisted out of the IP_MULTICAST gate so it is unconditionally available) so a high-bit top octet (>= 0x80) can no longer trigger ISO C11 6.5.7p4 undefined behavior on the int shift, with test_regression_dns_callback_high_bit_octet_ip_no_ub pinning the contract under -fsanitize=undefined.
1 parent 95bc67b commit b203126

3 files changed

Lines changed: 75 additions & 12 deletions

File tree

src/test/unit/unit.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,7 @@ Suite *wolf_suite(void)
394394
tcase_add_test(tc_utils, test_udp_try_recv_full_fifo_drop_does_not_set_readable_or_send_icmp);
395395
tcase_add_test(tc_utils, test_dns_callback_bad_flags);
396396
tcase_add_test(tc_utils, test_dns_callback_truncated_response_aborts_query);
397+
tcase_add_test(tc_utils, test_regression_dns_callback_high_bit_octet_ip_no_ub);
397398
tcase_add_test(tc_utils, test_dns_callback_bad_name);
398399
tcase_add_test(tc_utils, test_dns_callback_short_header_ignored);
399400
tcase_add_test(tc_utils, test_dns_callback_wrong_id_ignored);

src/test/unit/unit_tests_dns_dhcp.c

Lines changed: 65 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5187,6 +5187,71 @@ START_TEST(test_dns_callback_truncated_response_aborts_query)
51875187
}
51885188
END_TEST
51895189

5190+
START_TEST(test_regression_dns_callback_high_bit_octet_ip_no_ub)
5191+
{
5192+
/* The dns_callback() A-record reassembly used to compute
5193+
* ip = (buf[pos+3] & 0xFF) | ... | ((buf[pos+0] & 0xFF) << 24);
5194+
* with buf typed as char (signed by default). For the high-bit
5195+
* case (top octet >= 0x80) the int-typed (... & 0xFF) << 24
5196+
* produces a value that is not representable in int, which is
5197+
* undefined behavior per ISO C11 6.5.7p4. Under -fsanitize=undefined
5198+
* (make unit-ubsan) that shift trips a runtime error; this test
5199+
* pins the contract that high-bit IPs both (a) do not invoke UB
5200+
* and (b) are delivered to dns_lookup_cb byte-for-byte intact. */
5201+
struct wolfIP s;
5202+
uint8_t response[128];
5203+
int pos;
5204+
struct dns_header *hdr = (struct dns_header *)response;
5205+
struct dns_question *q;
5206+
struct dns_rr *rr;
5207+
const uint8_t ip_bytes[4] = {0xC8, 0x0A, 0x14, 0x1E}; /* 200.10.20.30 */
5208+
5209+
wolfIP_init(&s);
5210+
mock_link_init(&s);
5211+
s.dns_server = 0x0A000001U;
5212+
s.dns_query_type = DNS_QUERY_TYPE_A;
5213+
s.dns_id = 0x1234;
5214+
s.dns_lookup_cb = test_dns_lookup_cb;
5215+
dns_lookup_calls = 0;
5216+
dns_lookup_ip = 0;
5217+
s.dns_udp_sd = wolfIP_sock_socket(&s, AF_INET, IPSTACK_SOCK_DGRAM, WI_IPPROTO_UDP);
5218+
ck_assert_int_gt(s.dns_udp_sd, 0);
5219+
5220+
memset(response, 0, sizeof(response));
5221+
hdr->id = ee16(s.dns_id);
5222+
hdr->flags = ee16(0x8100);
5223+
hdr->qdcount = ee16(1);
5224+
hdr->ancount = ee16(1);
5225+
pos = sizeof(struct dns_header);
5226+
response[pos++] = 7; memcpy(&response[pos], "example", 7); pos += 7;
5227+
response[pos++] = 3; memcpy(&response[pos], "com", 3); pos += 3;
5228+
response[pos++] = 0;
5229+
q = (struct dns_question *)(response + pos);
5230+
q->qtype = ee16(DNS_A);
5231+
q->qclass = ee16(1);
5232+
pos += sizeof(struct dns_question);
5233+
response[pos++] = 0xC0;
5234+
response[pos++] = (uint8_t)sizeof(struct dns_header);
5235+
rr = (struct dns_rr *)(response + pos);
5236+
rr->type = ee16(DNS_A);
5237+
rr->class = ee16(1);
5238+
rr->ttl = ee32(60);
5239+
rr->rdlength = ee16(4);
5240+
pos += sizeof(struct dns_rr);
5241+
memcpy(&response[pos], ip_bytes, sizeof(ip_bytes));
5242+
pos += sizeof(ip_bytes);
5243+
5244+
enqueue_udp_rx(&s.udpsockets[SOCKET_UNMARK(s.dns_udp_sd)], response, (uint16_t)pos, DNS_PORT);
5245+
dns_callback(s.dns_udp_sd, CB_EVENT_READABLE, &s);
5246+
5247+
ck_assert_int_eq(dns_lookup_calls, 1);
5248+
ck_assert_uint_eq(dns_lookup_ip, 0xC80A141EU);
5249+
ck_assert_uint_eq(s.dns_id, 0);
5250+
ck_assert_int_eq(s.dns_query_type, DNS_QUERY_TYPE_NONE);
5251+
ck_assert_ptr_eq(s.dns_lookup_cb, NULL);
5252+
}
5253+
END_TEST
5254+
51905255
START_TEST(test_dns_callback_bad_name)
51915256
{
51925257
struct wolfIP s;

src/wolfip.c

Lines changed: 9 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1634,6 +1634,14 @@ static inline int wolfIP_ip_is_multicast(ip4 addr)
16341634
return ((addr & 0xF0000000U) == 0xE0000000U);
16351635
}
16361636

1637+
static uint32_t get_be32(const uint8_t *p)
1638+
{
1639+
uint32_t be;
1640+
1641+
memcpy(&be, p, sizeof(be));
1642+
return ee32(be);
1643+
}
1644+
16371645
#ifdef IP_MULTICAST
16381646
static uint16_t ip_checksum_buf(const void *buf, uint16_t len)
16391647
{
@@ -1664,14 +1672,6 @@ static void put_be32(uint8_t *p, uint32_t v)
16641672
memcpy(p, &be, sizeof(be));
16651673
}
16661674

1667-
static uint32_t get_be32(const uint8_t *p)
1668-
{
1669-
uint32_t be;
1670-
1671-
memcpy(&be, p, sizeof(be));
1672-
return ee32(be);
1673-
}
1674-
16751675
static void mcast_ip_to_eth(ip4 group, uint8_t mac[6])
16761676
{
16771677
mac[0] = 0x01;
@@ -8919,10 +8919,7 @@ void dns_callback(int dns_sd, uint16_t ev, void *arg)
89198919
dns_abort_query(s);
89208920
return;
89218921
}
8922-
ip = (buf[pos + 3] & 0xFF) |
8923-
((buf[pos + 2] & 0xFF) << 8) |
8924-
((buf[pos + 1] & 0xFF) << 16) |
8925-
((buf[pos + 0] & 0xFF) << 24);
8922+
ip = get_be32((const uint8_t *)buf + pos);
89268923
if (s->dns_lookup_cb)
89278924
s->dns_lookup_cb(ip);
89288925
dns_abort_query(s);

0 commit comments

Comments
 (0)