On Thu, Jul 09, 2015 at 11:16:05PM -0000, subashab@xxxxxxxxxxxxxx wrote:
> > This function is called from nf_nat_ipv4_fn(), see do_chain().
> >
> > And we're accepting the packet with no NAT mangling if we fail to add
> > the extension:
> >
> > nat = nf_ct_nat_ext_add(ct);
> > if (nat == NULL)
> > return NF_ACCEPT;
> >
> > Can you provide more information on what your static analysis software
> > reports? Thanks.
> >
>
> Sure, here is the report
>
> - In nf_nat_masquerade_ipv4.c line 40, 'nat' is assigned the value from
> function 'nfct_nat'
> - In nf_nat.h line 58, '__nf_ct_ext_find( (ct), (NF_CT_EXT_NAT) )' is
> assigned the return value from function '__nf_ct_ext_find'.
> - In nf_conntrack_extend.h line 68, '__nf_ct_ext_find' explicitly returns
> a NULL value.
>
> - As a result, pointer 'nat' returned from call to function 'nfct_nat' at
> line 40 may be NULL and may be dereferenced at line 59 'nat->masq_index =
> out->ifindex;'
I see, but if you look nf_nat_ipv4_fn() then you can confirm that we
always have a nat extension in place by when the iptables NAT
targets / nft NAT expressions:
nf_nat_ipv4_fn(...)
{
[...]
ct = nf_ct_get(skb, &ctinfo);
/* Can't track? It's not due to stress, or conntrack would
* have dropped it. Hence it's the user's responsibilty to
* packet filter it out, or implement conntrack/NAT for that
* protocol. 8) --RR
*/
if (!ct)
return NF_ACCEPT;
/* Don't try to NAT if this packet is not conntracked */
if (nf_ct_is_untracked(ct))
return NF_ACCEPT;
nat = nf_ct_nat_ext_add(ct);
if (nat == NULL)
return NF_ACCEPT;
...
If we fail to create the nat extension, then this accepts the packet,
so no chances we can reach this NULL dereference.
I wonder if this is a false positive. Would you please have a closer
look and confirm this? Thanks.
--
To unsubscribe from this list: send the line "unsubscribe netfilter-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
