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

[Ben Pfaff]

On Sat, May 20, 2017 at 09:01:34PM -0700, Greg Rose wrote:
> 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>

Thanks, I applied this to master and branch-2.7.