ipfilter: Verify ipnat on entry into kernel

[cy]

Summary

The ipnat struct is built by ipnat(8), specifically ipnat_y.y when
parsing the ipnat configuration file (typically ipnat.conf). ipnat
contains a variable length string field at the end of the struct. This
data field, called in_names, may contain various text strings such as
NIC names. There is no upper bound limit to the length of strings as
long as the in_namelen length field specifies the length of in_names
within the ipnat structure and in_size specifies the size of the ipnat
structure itself.

[markj]

sys/netpfil/ipfilter/netinet/ip_nat.c

1038

I would also verify that in_namelen >= 0. That might be slightly redundant with the tests above, but I'm not sure, given that both sides of sizeof(natd) + natd.in_namelen != natd.in_size will be promoted to unsigned types.

1070

Suppose _a->in_namelen == 0 and _a->_b is INT_MAX. Then _a->in_namelen < _a->_b, but v_in_toend >= 0.

It would be better to first check that _a->in_namelen >= _a->_b, then perform the subtraction if that test passes.

1078

Why are we doing all this work to validate that the strings don't overlap? Does that actually matter? I expect it would be sufficient to make sure that each offset is 1) within bounds of the structure and 2) is nul-terminated.

I posted an alternative version, D53805, which does the above. It avoids using a macro too, which I think makes the code easier to follow. It doesn't include the ipf_max_namelen check, the idea is just to show what I think is necessary and sufficient to fix potential memory safety issues. In particular, in my version it's possible for strings to overlap. What do you think?

[cy]

sys/netpfil/ipfilter/netinet/ip_nat.c

1078

This would work too but you should also use the same function in D53475. fp->fr_names is the same type of field as in.in_names.

[cy]

Replaced by D53843.