Re: [PATCH net] nfnetlink_osf: validate individual option lengths in fingerprints

[Florian Westphal]

bestswngs@xxxxxxxxx <bestswngs@xxxxxxxxx> wrote:
> From: Weiming Shi <bestswngs@xxxxxxxxx>
> 
> nfnl_osf_add_callback() validates opt_num bounds and string
> NUL-termination but does not check individual option length fields.
> A zero-length option causes nf_osf_match_one() to enter the option
> matching loop even when foptsize sums to zero, which matches packets
> with no TCP options where ctx->optp is NULL:

Would you mind if i squash:

diff --git a/net/netfilter/nfnetlink_osf.c b/net/netfilter/nfnetlink_osf.c
--- a/net/netfilter/nfnetlink_osf.c
+++ b/net/netfilter/nfnetlink_osf.c
@@ -302,6 +302,7 @@ static int nfnl_osf_add_callback(struct sk_buff *skb,
 {
        struct nf_osf_user_finger *f;
        struct nf_osf_finger *kf = NULL, *sf;
+       unsigned int tot_opt_len = 0;
        int err = 0;
        int i;

@@ -320,10 +321,14 @@ static int nfnl_osf_add_callback(struct sk_buff *skb,
                return -EINVAL;

        for (i = 0; i < f->opt_num; i++) {
-               if (!f->opt[i].length)
+               if (!f->opt[i].length || f->opt[i].length > MAX_IPOPTLEN)
                        return -EINVAL;
                if (f->opt[i].kind == OSFOPT_MSS && f->opt[i].length < 4)
                        return -EINVAL;
+
+               tot_opt_len += f->opt[i].length;
+               if (tot_opt_len > MAX_IPOPTLEN)
+                       return -EINVAL;
        }

        if (!memchr(f->genre, 0, MAXGENRELEN) ||

There is a runtime check (WTF) for this already, but arguably it
better belongs here.