[ovs-dev] [PATCH] pinctrl: Be more careful in parsing DHCPv6 and DNS.

[Greg Rose]

On Sat, 2017-05-20 at 16:57 -0700, Ben Pfaff wrote:
> pinctrl_handle_put_dhcpv6_opts() and pinctrl_handle_dns_lookup() were not
> checking that a full UDP header was present before reading its udp_len
> field.  This patch fixes the problem.
> 
> I don't think that the system as a whole, as normally installed, was
> exploitable.  This is because pinctrl processes a packet sent to it from
> ovs-vswitchd.  ovs-vswitchd only sends it UDPv6 DHCPv6 packets.  To
> determine that the packets are DHCPv6, ovs-vswitchd has to see its UDP port
> numbers are those for DHCPv6, and it's only going to see that if an entire
> UDP header is present.  Therefore, this part of pinctrl will only ever
> process a packet for which udp_len is there.
> 
> I believe that pinctrl_handle_dns_lookup() is similar.
> 
> Reported-by: Bhargava Shastry <bshastry at sec.t-labs.tu-berlin.de>
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
>  ovn/controller/pinctrl.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/ovn/controller/pinctrl.c b/ovn/controller/pinctrl.c
> index 9ad413376736..225f6a7563dc 100644
> --- a/ovn/controller/pinctrl.c
> +++ b/ovn/controller/pinctrl.c
> @@ -526,6 +526,11 @@ pinctrl_handle_put_dhcpv6_opts(
>  
>      struct udp_header *in_udp = dp_packet_l4(pkt_in);
>      const uint8_t *in_dhcpv6_data = dp_packet_get_udp_payload(pkt_in);
> +    if (!in_udp || !in_dhcpv6_data) {
> +        VLOG_WARN_RL(&rl, "truncated dhcpv6 packet");
> +        goto exit;
> +    }
> +
>      uint8_t out_dhcpv6_msg_type;
>      switch(*in_dhcpv6_data) {
>      case DHCPV6_MSG_TYPE_SOLICIT:
> @@ -710,6 +715,10 @@ pinctrl_handle_dns_lookup(
>  
>      /* Extract the DNS header */
>      struct dns_header const *in_dns_header = dp_packet_get_udp_payload(pkt_in);
> +    if (!in_dns_header) {
> +        VLOG_WARN_RL(&rl, "truncated dns packet");
> +        goto exit;
> +    }
>  
>      /* Check if it is DNS request or not */
>      if (in_dns_header->lo_flag & 0x80) {

LGTM

Acked-by: Greg Rose <gvrose8192 at gmail.com>