Re: [PATCH libmnl v2] nlmsg: fix false positives when validating buffer sizes

[Pablo Neira Ayuso <pablo@netfilter.org>]

On Sat, Nov 04, 2023 at 11:01:54PM +0000, Jeremy Sowden wrote:
> The `len` parameter of `mnl_nlmsg_ok`, which holds the buffer length and
> is compared to the size of the object expected to fit into the buffer,
> is signed because the function validates the length, and it can be
> negative in the case of malformed messages.  Comparing it to unsigned
> operands used to lead to compiler warnings:
> 
>   msg.c: In function 'mnl_nlmsg_ok':
>   msg.c:136: warning: comparison between signed and unsigned
>   msg.c:138: warning: comparison between signed and unsigned
> 
> and so commit 73661922bc3b ("fix warning in compilation due to different
> signess") added casts of the unsigned operands to `int`.  However, the
> comparison to `nlh->nlmsg_len`:
> 
>   (int)nlh->nlmsg_len <= len
> 
> is problematic, since `nlh->nlmsg_len` is of type `__u32` and so may
> hold values greater than `INT_MAX`.  In the case where `len` is positive
> and `nlh->nlmsg_len` is greater than `INT_MAX`, the cast will yield a
> negative value and `mnl_nlmsg_ok` will incorrectly return true.
> 
> Instead, assign `len` to an unsigned local variable, check for a
> negative value first, then use the unsigned local for the other
> comparisons, and remove the casts.

Applied, thanks Jeremy