public inbox for netfilter@vger.kernel.org
 help / color / mirror / Atom feed
* Re: [SECURITY] nf_tables: incorrect sscanf return check leads to use of uninitialized variable
       [not found] <CALGDAeBqaTDJhZr0AuFz5M2fmgXsyxLz73Rkqj5ZwwRFoFHoGg@mail.gmail.com>
@ 2025-12-25 22:19 ` Pablo Neira Ayuso
  0 siblings, 0 replies; only message in thread
From: Pablo Neira Ayuso @ 2025-12-25 22:19 UTC (permalink / raw)
  To: Maiquel Paiva; +Cc: security, netfilter, coreteam

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!

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2025-12-25 22:19 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
     [not found] <CALGDAeBqaTDJhZr0AuFz5M2fmgXsyxLz73Rkqj5ZwwRFoFHoGg@mail.gmail.com>
2025-12-25 22:19 ` [SECURITY] nf_tables: incorrect sscanf return check leads to use of uninitialized variable Pablo Neira Ayuso

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox