Re: [SECURITY] nf_tables: incorrect sscanf return check leads to use of uninitialized variable

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

Hi,

On Thu, Dec 25, 2025 at 06:06:29PM -0300, Maiquel Paiva wrote:
> Summary
> -------
> nf_tables_set_alloc_name() uses an incorrect return-value check for
> sscanf(),
> which may lead to the use of an uninitialized stack variable.
> 
> Affected code
> -------------
> File: net/netfilter/nf_tables_api.c
> Function: nf_tables_set_alloc_name()
> 
> Relevant snippet:
> 
>     list_for_each_entry(i, &ctx->table->sets, list) {
>         int tmp;
> 
>         if (!nft_is_active_next(ctx->net, i))
>             continue;
>         if (!sscanf(i->name, name, &tmp))
>             continue;
>         if (tmp < min || tmp >= min + BITS_PER_BYTE * PAGE_SIZE)
>             continue;
> 
>         set_bit(tmp - min, inuse);
>     }
> 
> Problem description
> -------------------
> sscanf() returns the number of successfully assigned input items, or EOF
> (-1)
> if an input failure occurs before any conversion.
> 
> The current check:
> 
>     if (!sscanf(...))
> 
> only rejects the case where sscanf() returns 0. If sscanf() returns -1
> (EOF),
> the condition evaluates to false, and the code continues execution with
> `tmp`
> left uninitialized.

Looking at lib/vsprintf.c, I don't see how this can return -1.

And you will have to fix more code in the kernel if your statement
would be true:

net/core/dev.c:                 if (!sscanf(name_node->name, name, &i))

> This may lead to undefined behavior when `tmp` is later used in arithmetic
> and as an index for set_bit().

Even if that would true, tmp is checked to be on the boundaries right
after this.

                        if (!sscanf(i->name, name, &tmp))
                                continue;
                        if (tmp < min || tmp >= min + BITS_PER_BYTE * PAGE_SIZE) <--- here
                                continue;

> Proof of incorrect check
> ------------------------
> A simple user-space test demonstrates that sscanf() returns -1 for empty
> or whitespace-only strings:
> 
>     input: ""      -> sscanf return = -1
>     input: "   "   -> sscanf return = -1
>     input: "abc"   -> sscanf return = 0
>     input: "123"   -> sscanf return = 1
> 
> In the -1 case, the current kernel code does not execute the `continue`
> statement and uses an uninitialized `tmp`.
> 
> Impact
> ------
> Depending on stack contents, this may result in out-of-bounds bit
> operations,
> memory corruption, or kernel crashes (DoS). While this is a logic bug, it
> has
> security implications.

No. This report is bullshit.

Happy holidays!