Skip to content

Commit

Permalink
pinctrl: Fix DNS packet parsing
Browse files Browse the repository at this point in the history
Due to the use of a uint8_t to index inside the DNS payload we could end
up in an infinite loop when specific (invalid) DNS packets were
processed by ovn-controller. In the infinite loop we keep increasing the
query_name dynamic string until running out of memory.

One way to replicate the issue is to configure DNS on the logical switch
and then inject a manually crafted DNS-like packet. For example, with
Scapy:

>>> p = IP(dst='10.0.0.2',src='10.0.0.3')/UDP(dport=53)/('a'*364)
>>> send(p)

Also add a sanity check on minimum L4 size of packets.

(cherry-picked from ovn commit - 7fbdeaa)

CC: Numan Siddique <nusiddiq@redhat.com>
Fixes: 16cb4fb ("ovn-controller: Add 'dns_lookup' action")
Reported-at: https://bugzilla.redhat.com/1740335
Reported-by: Priscila <pveiga@redhat.com>
Signed-off-by: Dumitru Ceara <dceara@redhat.com>
Signed-off-by: Numan Siddique <nusiddiq@redhat.com>
Signed-off-by: Ben Pfaff <blp@ovn.org>
  • Loading branch information
dceara authored and blp committed Aug 28, 2019
1 parent 52b06b5 commit 8daa479
Showing 1 changed file with 7 additions and 1 deletion.
8 changes: 7 additions & 1 deletion ovn/controller/pinctrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -1614,6 +1614,12 @@ pinctrl_handle_dns_lookup(
goto exit;
}

/* Check that the packet stores at least the minimal headers. */
if (dp_packet_l4_size(pkt_in) < (UDP_HEADER_LEN + DNS_HEADER_LEN)) {
VLOG_WARN_RL(&rl, "truncated dns packet");
goto exit;
}

/* Extract the DNS header */
struct dns_header const *in_dns_header = dp_packet_get_udp_payload(pkt_in);
if (!in_dns_header) {
Expand All @@ -1638,7 +1644,7 @@ pinctrl_handle_dns_lookup(
uint8_t *end = (uint8_t *)in_udp + MIN(udp_len, l4_len);
uint8_t *in_dns_data = (uint8_t *)(in_dns_header + 1);
uint8_t *in_queryname = in_dns_data;
uint8_t idx = 0;
uint16_t idx = 0;
struct ds query_name;
ds_init(&query_name);
/* Extract the query_name. If the query name is - 'www.ovn.org' it would be
Expand Down

0 comments on commit 8daa479

Please sign in to comment.