[ovs-git] [openvswitch/ovs] 5b2cdc: flow: Verify that tot_len >= ip_len in miniflow_ex...

[GitHub]

  Branch: refs/heads/master
  Home:   https://github.com/openvswitch/ovs
  Commit: 5b2cdc3fbbad739c09f47f420cd4f1c54c0399fb
      https://github.com/openvswitch/ovs/commit/5b2cdc3fbbad739c09f47f420cd4f1c54c0399fb
  Author: Ben Pfaff <blp at ovn.org>
  Date:   2016-07-26 (Tue, 26 Jul 2016)

  Changed paths:
    M lib/flow.c

  Log Message:
  -----------
  flow: Verify that tot_len >= ip_len in miniflow_extract().

miniflow_extract() uses the following quantities when it examines an IPv4
header:

    size, the number of bytes from the start of the IPv4 header onward
    ip_len, the number of bytes in the IPv4 header (from the IHL field)
    tot_len, same as size but taken from IPv4 header Total Length field

Until now, the code in miniflow_extract() verified these invariants:

    size >= 20 (minimum IP header length)
    ip_len >= 20 (ditto)
    ip_len <= size (to avoid reading past end of packet)
    tot_len <= size (ditto)
    size - tot_len <= 255 (because this is stored in a 1-byte variable
			   internally and wouldn't normally be big)

It failed to verify the following, which is not implied by the conjunction
of the above:

    ip_len <= tot_len (e.g. that the IP header fits in the packet)

This means that the code was willing to read past the end of an IP
packet's declared length, up to the actual end of the packet including any
L2 padding.  For example, given:

    size = 44
    ip_len = 44
    tot_len = 40

miniflow_extract() would successfully verify all the constraints, then:

    * Trim off 4 bytes of tail padding (size - tot_len), reducing size to
      40 to match tot_len.
    * Pull 44 (ip_len) bytes of IP header, even though there are only 40
      bytes left.  This causes 'size' to wrap around to SIZE_MAX-4.

Given an IP protocol that OVS understands (such as TCP or UDP), this
integer wraparound could cause OVS to read past the end of the packet.
In turn, this could cause OVS to extract invalid port numbers, TCP flags,
or ICMPv4 or ICMPv6 or IGMP type and code from arbitrary heap data
past the end of a packet.

This bug has common hallmarks of a security vulnerability, but we do not
know of a way to exploit this bug to cause an Open vSwitch crash, or to
extract sensitive data from Open vSwitch address space to an attacker's
benefit.

We do not have a specific example, but it is reasonable to suspect that
this bug could allow an attacker in some circumstances to bypass ACLs
implemented via Open vSwitch flow tables.  However, any IP packet that
triggers this bug is invalid and should be rejected in an early stage of a
receiver's IP stack.  For the same reason, any IP packet that triggers this
bug will also be dropped by any IP router, so an attacker would have to
share the same L2 segment as the victim.  In conjunction with an IP stack
that has a similar bug, of course, this could cause some damage, but we do
not know of an IP stack with such a bug; neither Linux nor the OVS
userspace tunnel implementation appear to have such a bug.

Reported-by: Bhargava Shastry <bshastry at sec.t-labs.tu-berlin.de>
Reported-by: Kashyap Thimmaraju <Kashyap.Thimmaraju at sec.t-labs.tu-berlin.de>
Signed-off-by: Ben Pfaff <blp at ovn.org>
Acked-by: Flavio Leitner <fbl at sysclose.org>