Re: CVS commit: src/sys/net/npf

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

"Maxime Villard" <maxv%netbsd.org@localhost> wrote:
> Module Name:	src
> Committed By:	maxv
> Date:		Fri Dec 15 21:00:26 UTC 2017
> 
> Modified Files:
> 	src/sys/net/npf: npf.h
> 
> Log Message:
> Fix a vulnerability in NPF, that allows whatever incoming IPv6 packet to
> bypass a certain number of filtering rules.
> 
> Basically there is an integer overflow in npf_cache_ip: npc_hlen is a
> 8bit unsigned int, and can wrap to zero if the IPv6 packet being processed
> has large extensions.

Thanks for discovering and fixing this.  I think this is the first
serious remote vulnerability in NPF, although limited to IPv6 only.

> Fix this by using uint32_t. While here, it seems to me there is also a
> memory overflow: still in npf_cache_ip, npc_hlen may be incremented with
> a value that goes beyond the mbuf.

A minor aspect, but promoting npf_hlen to uint32_t results in wasteful
padding in the struct, so it is better to re-order the struct members
in this case.

If the npc_hlen value is beyond the packet length, NPF's nbuf interface
will catch that, since it performs the bounds check.  However, I think
we should implement some sanity check for the npc_hlen value.  RFC 7112
suggests that the IPv6 header chain should not exceed the MTU size (and
thus fit in the first fragment, in case of fragmentation).  Some value
along these lines could be the basis for a sanity check..

-- 
Mindaugas