[ovs-dev] [PATCH] flow: Verify that tot_len >= ip_len in miniflow_extract().

[Flavio Leitner]

On Fri, Jul 22, 2016 at 04:43:50PM -0700, Ben Pfaff wrote:
> 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>