[PATCH] netfilter: ipv4: Fix use-after-free in send_reset

[PATCH] netfilter: ipv4: Fix use-after-free in send_reset

[Tejaswi Tanikella]

niph is not updated after pskb_expand_head changes the skb head.
It still points to the freed data, which is then used to update
tot_len and checksum. This could cause use-after-free poison crash.

Update niph, if ip_route_me_harder does not fail.

Signed-off-by: Tejaswi Tanikella <tejaswit@codeaurora.org>
---
 net/ipv4/netfilter/nf_reject_ipv4.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
index eeacbda..b2e0f90 100644
--- a/net/ipv4/netfilter/nf_reject_ipv4.c
+++ b/net/ipv4/netfilter/nf_reject_ipv4.c
@@ -131,6 +131,8 @@ void nf_send_reset(struct net *net, struct sk_buff *oldskb, int hook)
 
 	if (ip_route_me_harder(net, nskb, RTN_UNSPEC))
 		goto free_nskb;
+	else
+		niph = ip_hdr(nskb);
 
 	/* "Never happens" */
 	if (nskb->len > dst_mtu(skb_dst(nskb)))
-- 
1.9.1

Re: [PATCH] netfilter: ipv4: Fix use-after-free in send_reset

[Pablo Neira Ayuso]

On Mon, Oct 23, 2017 at 11:46:03AM +0530, Tejaswi Tanikella wrote:
> niph is not updated after pskb_expand_head changes the skb head.
> It still points to the freed data, which is then used to update
> tot_len and checksum. This could cause use-after-free poison crash.
> 
> Update niph, if ip_route_me_harder does not fail.
> 
> Signed-off-by: Tejaswi Tanikella <tejaswit@codeaurora.org>
> ---
>  net/ipv4/netfilter/nf_reject_ipv4.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
> index eeacbda..b2e0f90 100644
> --- a/net/ipv4/netfilter/nf_reject_ipv4.c
> +++ b/net/ipv4/netfilter/nf_reject_ipv4.c
> @@ -131,6 +131,8 @@ void nf_send_reset(struct net *net, struct sk_buff *oldskb, int hook)
>  
>  	if (ip_route_me_harder(net, nskb, RTN_UNSPEC))
>  		goto free_nskb;
> +	else
> +		niph = ip_hdr(nskb);

No need for else here, right? So we can just turn this into a one
liner to inconditionally reload this pointer.

>  	/* "Never happens" */
>  	if (nskb->len > dst_mtu(skb_dst(nskb)))
> -- 
> 1.9.1
> 

Re: [PATCH] netfilter: ipv4: Fix use-after-free in send_reset

[Pablo Neira Ayuso]

On Wed, Nov 01, 2017 at 01:36:36AM +0100, Pablo Neira Ayuso wrote:
> On Mon, Oct 23, 2017 at 11:46:03AM +0530, Tejaswi Tanikella wrote:
> > niph is not updated after pskb_expand_head changes the skb head.
> > It still points to the freed data, which is then used to update
> > tot_len and checksum. This could cause use-after-free poison crash.
> > 
> > Update niph, if ip_route_me_harder does not fail.
> > 
> > Signed-off-by: Tejaswi Tanikella <tejaswit@codeaurora.org>
> > ---
> >  net/ipv4/netfilter/nf_reject_ipv4.c | 2 ++
> >  1 file changed, 2 insertions(+)
> > 
> > diff --git a/net/ipv4/netfilter/nf_reject_ipv4.c b/net/ipv4/netfilter/nf_reject_ipv4.c
> > index eeacbda..b2e0f90 100644
> > --- a/net/ipv4/netfilter/nf_reject_ipv4.c
> > +++ b/net/ipv4/netfilter/nf_reject_ipv4.c
> > @@ -131,6 +131,8 @@ void nf_send_reset(struct net *net, struct sk_buff *oldskb, int hook)
> >  
> >  	if (ip_route_me_harder(net, nskb, RTN_UNSPEC))
> >  		goto free_nskb;
> > +	else
> > +		niph = ip_hdr(nskb);
> 
> No need for else here, right? So we can just turn this into a one
> liner to inconditionally reload this pointer.

I'm just going to remove the 'else' and push out this.