Messages in this thread | | | Subject | Re: [PATCH] ipv6: Prevent overrun when parsing v6 header options | From | Eric Dumazet <> | Date | Fri, 31 May 2019 10:35:20 -0700 |
| |
On 5/30/19 8:04 PM, Yang Xiao wrote: > On Fri, May 31, 2019 at 1:17 AM Eric Dumazet <eric.dumazet@gmail.com> wrote: >> >> >> >> On 5/30/19 8:28 AM, Young Xiao wrote: >>> The fragmentation code tries to parse the header options in order >>> to figure out where to insert the fragment option. Since nexthdr points >>> to an invalid option, the calculation of the size of the network header >>> can made to be much larger than the linear section of the skb and data >>> is read outside of it. >>> >>> This vulnerability is similar to CVE-2017-9074. >>> >>> Signed-off-by: Young Xiao <92siuyang@gmail.com> >>> --- >>> net/ipv6/mip6.c | 24 ++++++++++++++---------- >>> 1 file changed, 14 insertions(+), 10 deletions(-) >>> >>> diff --git a/net/ipv6/mip6.c b/net/ipv6/mip6.c >>> index 64f0f7b..30ed1c5 100644 >>> --- a/net/ipv6/mip6.c >>> +++ b/net/ipv6/mip6.c >>> @@ -263,8 +263,6 @@ static int mip6_destopt_offset(struct xfrm_state *x, struct sk_buff *skb, >>> u8 **nexthdr) >>> { >>> u16 offset = sizeof(struct ipv6hdr); >>> - struct ipv6_opt_hdr *exthdr = >>> - (struct ipv6_opt_hdr *)(ipv6_hdr(skb) + 1); >>> const unsigned char *nh = skb_network_header(skb); >>> unsigned int packet_len = skb_tail_pointer(skb) - >>> skb_network_header(skb); >>> @@ -272,7 +270,8 @@ static int mip6_destopt_offset(struct xfrm_state *x, struct sk_buff *skb, >>> >>> *nexthdr = &ipv6_hdr(skb)->nexthdr; >>> >>> - while (offset + 1 <= packet_len) { >>> + while (offset <= packet_len) { >>> + struct ipv6_opt_hdr *exthdr; >>> >>> switch (**nexthdr) { >>> case NEXTHDR_HOP: >>> @@ -299,12 +298,15 @@ static int mip6_destopt_offset(struct xfrm_state *x, struct sk_buff *skb, >>> return offset; >>> } >>> >>> + if (offset + sizeof(struct ipv6_opt_hdr) > packet_len) >>> + return -EINVAL; >>> + >>> + exthdr = (struct ipv6_opt_hdr *)(nh + offset); >>> offset += ipv6_optlen(exthdr); >>> *nexthdr = &exthdr->nexthdr; >>> - exthdr = (struct ipv6_opt_hdr *)(nh + offset); >>> } >>> >>> - return offset; >>> + return -EINVAL; >>> } >>> >> >> >> Ok, but have you checked that callers have been fixed ? > > I've checked the callers. There are two callers: > xfrm6_transport_output() and xfrm6_ro_output(). There are checks in > both function. > > ------------------------------------------------------------------------------ > hdr_len = x->type->hdr_offset(x, skb, &prevhdr); > if (hdr_len < 0) > return hdr_len; > ------------------------------------------------------------------------------ >> >> xfrm6_transport_output() seems buggy as well, >> unless the skbs are linearized before entering these functions ? > I can not understand what you mean about this comment. > Could you explain it in more detail.
If we had a problem, then the memmove(ipv6_hdr(skb), iph, hdr_len); in xfrm6_transport_output() would be buggy, since iph could also point to freed memory.
| |