On Fri, Aug 16, 2019 at 2:31 PM Numan Siddique <[email protected]> wrote:
>
>
>
> On Fri, Aug 16, 2019 at 4:15 PM Dumitru Ceara <[email protected]> wrote:
>>
>> 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.
>>
>> CC: Numan Siddique <[email protected]>
>> Fixes: 16cb4fb8ca49 ("ovn-controller: Add 'dns_lookup' action")
>> Reported-at: https://bugzilla.redhat.com/1740335
>> Reported-by: Priscila <[email protected]>
>> Signed-off-by: Dumitru Ceara <[email protected]>
>> ---
>
>
> Thanks for the fix Dumitru. I applied this patch to master.
> Can you please backport it to 2.12 (and other branches if you think it's
> required).
>
> Thanks
> Numan
Thanks for applying this. Yes, this should be backported at least down
to 2.9. I'll send backport patches soon.
Cheers,
Dumitru
>
>>
>> controller/pinctrl.c | 8 +++++++-
>> 1 file changed, 7 insertions(+), 1 deletion(-)
>>
>> diff --git a/controller/pinctrl.c b/controller/pinctrl.c
>> index f27718f..365a0d1 100644
>> --- a/controller/pinctrl.c
>> +++ b/controller/pinctrl.c
>> @@ -1628,6 +1628,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) {
>> @@ -1652,7 +1658,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
>> --
>> 1.8.3.1
>>
_______________________________________________
dev mailing list
[email protected]
https://mail.openvswitch.org/mailman/listinfo/ovs-dev
