Linux Netfilter development
 help / color / mirror / Atom feed
* Global buffer overflow in parse_ip6_mask()
@ 2025-12-18 13:15 Ilia Kashintsev
  2025-12-18 14:44 ` Phil Sutter
  2025-12-20 21:11 ` Pablo Neira Ayuso
  0 siblings, 2 replies; 5+ messages in thread
From: Ilia Kashintsev @ 2025-12-18 13:15 UTC (permalink / raw)
  To: netfilter-devel

Hello maintainers! I have found several global buffer overflows in
useful_functions.c

They both occur in the function parse_ip6_mask() and are caused by
unconditionally writing to p[16].

The first overflow occurs when bits is equal to 128,
which causes p[bits / 8] = 0xff << (8 - (bits & 7)); to write at p[16].

The second overflow occurs when bits is equal to 8,
which causes memset(p + (bits / 8) + 1, 0, (128 - bits) / 8); to write
15 bytes starting at p + 2, which leads to the same issue.

Sanitizer output for the first overflow:

==4013==ERROR: AddressSanitizer: global-buffer-overflow on address
0x55972a77b5f0 at pc 0x559729dd1d13 bp 0x7ffed4db8810 sp
0x7ffed4db8808
WRITE of size 1 at 0x55972a77b5f0 thread T0
    #0 0x559729dd1d12 in parse_ip6_mask
/orig/pkg-ebtables/useful_functions.c:368:15
    #1 0x559729dd18e7 in ebt_parse_ip6_address
/orig/pkg-ebtables/useful_functions.c:392:14
    #2 0x559729dcfdc2 in main /orig/pkg-ebtables/reproduction.c:40:5
    #3 0x7f6d398ee249 in __libc_start_call_main
csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #4 0x7f6d398ee304 in __libc_start_main csu/../csu/libc-start.c:360:3
    #5 0x559729cf0450 in _start
(/orig/pkg-ebtables/reproduction+0x2f450) (BuildId:
b1b8105cc025dc9ed11bd734af2fe28338caec61)

0x55972a77b5f0 is located 16 bytes before global variable
'numeric_to_addr.ap' defined in
'/orig/pkg-ebtables/useful_functions.c:341' (0x55972a77b600) of size
16
0x55972a77b5f0 is located 0 bytes after global variable
'parse_ip6_mask.maskaddr' defined in
'/orig/pkg-ebtables/useful_functions.c:351' (0x55972a77b5e0) of size
16
SUMMARY: AddressSanitizer: global-buffer-overflow
/orig/pkg-ebtables/useful_functions.c:368:15 in parse_ip6_mask
Shadow bytes around the buggy address:
  0x55972a77b300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x55972a77b380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x55972a77b400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x55972a77b480: 00 00 00 00 04 f9 f9 f9 04 f9 f9 f9 00 00 04 f9
  0x55972a77b500: f9 f9 f9 f9 00 00 00 00 00 00 03 f9 f9 f9 f9 f9
=>0x55972a77b580: 00 00 00 00 00 00 04 f9 f9 f9 f9 f9 00 00[f9]f9
  0x55972a77b600: 00 00 f9 f9 00 00 00 00 00 f9 f9 f9 04 f9 f9 f9
  0x55972a77b680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x55972a77b700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x55972a77b780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x55972a77b800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==4013==ABORTING


Sanitizer output for the second overflow:

==4011==ERROR: AddressSanitizer: global-buffer-overflow on address
0x633d6f5115f0 at pc 0x633d6eb23705 bp 0x7ffc5604fd70 sp
0x7ffc5604f540
WRITE of size 15 at 0x633d6f5115f0 thread T0
    #0 0x633d6eb23704 in __asan_memset
(/orig/pkg-ebtables/reproduction+0xcc704) (BuildId:
b1b8105cc025dc9ed11bd734af2fe28338caec61)
    #1 0x633d6eb67cb6 in parse_ip6_mask
/orig/pkg-ebtables/useful_functions.c:367:3
    #2 0x633d6eb678e7 in ebt_parse_ip6_address
/orig/pkg-ebtables/useful_functions.c:392:14
    #3 0x633d6eb65dc2 in main /orig/pkg-ebtables/reproduction.c:40:5
    #4 0x7e04737e6249 in __libc_start_call_main
csu/../sysdeps/nptl/libc_start_call_main.h:58:16
    #5 0x7e04737e6304 in __libc_start_main csu/../csu/libc-start.c:360:3
    #6 0x633d6ea86450 in _start
(/orig/pkg-ebtables/reproduction+0x2f450) (BuildId:
b1b8105cc025dc9ed11bd734af2fe28338caec61)

0x633d6f5115f0 is located 16 bytes before global variable
'numeric_to_addr.ap' defined in
'/orig/pkg-ebtables/useful_functions.c:341' (0x633d6f511600) of size
16
0x633d6f5115f0 is located 0 bytes after global variable
'parse_ip6_mask.maskaddr' defined in
'/orig/pkg-ebtables/useful_functions.c:351' (0x633d6f5115e0) of size
16
SUMMARY: AddressSanitizer: global-buffer-overflow
(/orig/pkg-ebtables/reproduction+0xcc704) (BuildId:
b1b8105cc025dc9ed11bd734af2fe28338caec61) in __asan_memset
Shadow bytes around the buggy address:
  0x633d6f511300: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x633d6f511380: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x633d6f511400: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x633d6f511480: 00 00 00 00 04 f9 f9 f9 04 f9 f9 f9 00 00 04 f9
  0x633d6f511500: f9 f9 f9 f9 00 00 00 00 00 00 03 f9 f9 f9 f9 f9
=>0x633d6f511580: 00 00 00 00 00 00 04 f9 f9 f9 f9 f9 00 00[f9]f9
  0x633d6f511600: 00 00 f9 f9 00 00 00 00 00 f9 f9 f9 04 f9 f9 f9
  0x633d6f511680: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x633d6f511700: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x633d6f511780: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
  0x633d6f511800: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
Shadow byte legend (one shadow byte represents 8 application bytes):
  Addressable:           00
  Partially addressable: 01 02 03 04 05 06 07
  Heap left redzone:       fa
  Freed heap region:       fd
  Stack left redzone:      f1
  Stack mid redzone:       f2
  Stack right redzone:     f3
  Stack after return:      f5
  Stack use after scope:   f8
  Global redzone:          f9
  Global init order:       f6
  Poisoned by user:        f7
  Container overflow:      fc
  Array cookie:            ac
  Intra object redzone:    bb
  ASan internal:           fe
  Left alloca redzone:     ca
  Right alloca redzone:    cb
==4011==ABORTING


Reproduction:
1) Build the project with sanitizers:

export CFLAGS="-g -O0 -fsanitize=address"
export CXXFLAGS="-g -O0 -fsanitize=address"
export CC=clang
export CXX=clang++

autoreconf -fi
./configure --enable-static --disable-shared
make

2) Build the example:

clang -g -O0 -fsanitize=address -o reproduction reproduction.c
libebtc_la-useful_functions.o ./.libs/libebtc.a

reproduction.c:

#include <stdio.h>
#include <unistd.h>

#include <arpa/inet.h>
#include <netinet/in.h>

#if defined(__linux__)
#  include <netinet/ether.h>
#else
#  include <net/ethernet.h>
#  include <arpa/inet.h>
#  include <netinet/ether.h>
#endif

void ebt_parse_ip6_address(char *address, struct in6_addr *addr,
struct in6_addr *msk);

int main(void) {
    unsigned char buf[256];
    ssize_t len;

    len = read(STDIN_FILENO, buf, sizeof(buf));
    if (len <= 0) {
        if (len == 0) {
            return 0;
        } else {
            perror("read");
            return 1;
        }
    }

    if (len < sizeof(buf)) {
        buf[len] = '\0';
    } else {
        buf[sizeof(buf) - 1] = '\0';
    }

    struct in6_addr addr = {0};
    struct in6_addr msk = {0};

    ebt_parse_ip6_address((char *)buf, &addr, &msk);

    return 0;
}


3) Launch the example with the provided inputs:

./reproduction < input1.txt
./reproduction < input2.txt

To generate input1.txt and input2.txt copy the corresponding text into
bs64_1.txt and bs64_2.txt and then run:

base64 -d bs64_1.txt > input1.txt
base64 -d bs64_2.txt > input2.txt

bs64_1.txt:
WTEvLzESES8vMTI4AAAAWQAAAAExEhIfAIAADgBk/wB/8Q4mLwAAAAAAAAEAMTIxDg4=

bs64_2.txt:
WWxsN4BvUTYg2GxsLzgADg==


Suggested fix:

It is proposed to handle 2 cases separately, keeping current behaviour
when bits % 8 != 0 and avoiding it when bits % 8 == 0. Nevertheless, i
can't be sure this fix is totally correct.

diff --git a/useful_functions.c b/useful_functions.c
index 133ae2f..a8dfcbc 100644
--- a/useful_functions.c
+++ b/useful_functions.c
@@ -364,8 +364,12 @@ static struct in6_addr *parse_ip6_mask(char *mask)
        if (bits != 0) {
                char *p = (char *)&maskaddr;
                memset(p, 0xff, bits / 8);
-               memset(p + (bits / 8) + 1, 0, (128 - bits) / 8);
-               p[bits / 8] = 0xff << (8 - (bits & 7));
+               if (bits & 7) {
+                       memset(p + (bits / 8) + 1, 0, (128 - bits) / 8);
+                       p[bits / 8] = 0xff << (8 - (bits & 7));
+               } else {
+                       memset(p + (bits / 8), 0, (128 - bits) / 8);
+               }
                return &maskaddr;
        }

^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: Global buffer overflow in parse_ip6_mask()
  2025-12-18 13:15 Global buffer overflow in parse_ip6_mask() Ilia Kashintsev
@ 2025-12-18 14:44 ` Phil Sutter
  2026-02-11 13:08   ` Florian Westphal
  2025-12-20 21:11 ` Pablo Neira Ayuso
  1 sibling, 1 reply; 5+ messages in thread
From: Phil Sutter @ 2025-12-18 14:44 UTC (permalink / raw)
  To: Ilia Kashintsev; +Cc: netfilter-devel

Hi,

On Thu, Dec 18, 2025 at 04:15:05PM +0300, Ilia Kashintsev wrote:
> Hello maintainers! I have found several global buffer overflows in
> useful_functions.c
> 
> They both occur in the function parse_ip6_mask() and are caused by
> unconditionally writing to p[16].
> 
> The first overflow occurs when bits is equal to 128,
> which causes p[bits / 8] = 0xff << (8 - (bits & 7)); to write at p[16].
> 
> The second overflow occurs when bits is equal to 8,
> which causes memset(p + (bits / 8) + 1, 0, (128 - bits) / 8); to write
> 15 bytes starting at p + 2, which leads to the same issue.

Funny how broken the code is, given its age.

[...]
> diff --git a/useful_functions.c b/useful_functions.c
> index 133ae2f..a8dfcbc 100644
> --- a/useful_functions.c
> +++ b/useful_functions.c
> @@ -364,8 +364,12 @@ static struct in6_addr *parse_ip6_mask(char *mask)
>         if (bits != 0) {
>                 char *p = (char *)&maskaddr;
>                 memset(p, 0xff, bits / 8);
> -               memset(p + (bits / 8) + 1, 0, (128 - bits) / 8);
> -               p[bits / 8] = 0xff << (8 - (bits & 7));
> +               if (bits & 7) {
> +                       memset(p + (bits / 8) + 1, 0, (128 - bits) / 8);
> +                       p[bits / 8] = 0xff << (8 - (bits & 7));
> +               } else {
> +                       memset(p + (bits / 8), 0, (128 - bits) / 8);
> +               }
>                 return &maskaddr;
>         }

The reason why the second memset() call may mis-behave is the broken
div-round-up in there: It does (bits / 8) + 1 when it should do
(bits + 7) / 8 instead. Fixed that, only the p[bits / 8] field access
needs to remain conditional:

@@ -364,8 +364,9 @@ static struct in6_addr *parse_ip6_mask(char *mask)
        if (bits != 0) {
                char *p = (char *)&maskaddr;
                memset(p, 0xff, bits / 8);
-               memset(p + (bits / 8) + 1, 0, (128 - bits) / 8);
-               p[bits / 8] = 0xff << (8 - (bits & 7));
+               memset(p + (bits + 7) / 8, 0, (128 - bits) / 8);
+               if (bits & 7)
+                       p[bits / 8] = 0xff << (8 - (bits & 7));
                return &maskaddr;
        }

What do you think?

Cheers, Phil

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Global buffer overflow in parse_ip6_mask()
  2025-12-18 13:15 Global buffer overflow in parse_ip6_mask() Ilia Kashintsev
  2025-12-18 14:44 ` Phil Sutter
@ 2025-12-20 21:11 ` Pablo Neira Ayuso
  1 sibling, 0 replies; 5+ messages in thread
From: Pablo Neira Ayuso @ 2025-12-20 21:11 UTC (permalink / raw)
  To: Ilia Kashintsev; +Cc: netfilter-devel

On Thu, Dec 18, 2025 at 04:15:05PM +0300, Ilia Kashintsev wrote:
[...]
> 2) Build the example:
> 
> clang -g -O0 -fsanitize=address -o reproduction reproduction.c
> libebtc_la-useful_functions.o ./.libs/libebtc.a
> 
> reproduction.c:

Hm, a custom file that uses this static library?
Maybe you describe how to crash this using ebtables userspace tool?
This is not even a shared library system wide available?

More comments below.

> #include <stdio.h>
> #include <unistd.h>
> 
> #include <arpa/inet.h>
> #include <netinet/in.h>
> 
> #if defined(__linux__)
> #  include <netinet/ether.h>
> #else
> #  include <net/ethernet.h>
> #  include <arpa/inet.h>
> #  include <netinet/ether.h>
> #endif
> 
> void ebt_parse_ip6_address(char *address, struct in6_addr *addr,
> struct in6_addr *msk);
> 
> int main(void) {
>     unsigned char buf[256];
>     ssize_t len;
> 
>     len = read(STDIN_FILENO, buf, sizeof(buf));
>     if (len <= 0) {
>         if (len == 0) {
>             return 0;
>         } else {
>             perror("read");
>             return 1;
>         }
>     }
> 
>     if (len < sizeof(buf)) {
>         buf[len] = '\0';
>     } else {
>         buf[sizeof(buf) - 1] = '\0';
>     }
> 
>     struct in6_addr addr = {0};
>     struct in6_addr msk = {0};
> 
>     ebt_parse_ip6_address((char *)buf, &addr, &msk);
> 
>     return 0;
> }
> 
> 
> 3) Launch the example with the provided inputs:
> 
> ./reproduction < input1.txt
> ./reproduction < input2.txt
> 
> To generate input1.txt and input2.txt copy the corresponding text into
> bs64_1.txt and bs64_2.txt and then run:
> 
> base64 -d bs64_1.txt > input1.txt
> base64 -d bs64_2.txt > input2.txt
> 
> bs64_1.txt:
> WTEvLzESES8vMTI4AAAAWQAAAAExEhIfAIAADgBk/wB/8Q4mLwAAAAAAAAEAMTIxDg4=
> 
> bs64_2.txt:
> WWxsN4BvUTYg2GxsLzgADg==

Maybe next time just post a patch to fix this silly bug... this tool
can only be run with root priviledges.

> Suggested fix:
> 
> It is proposed to handle 2 cases separately, keeping current behaviour
> when bits % 8 != 0 and avoiding it when bits % 8 == 0. Nevertheless, i
> can't be sure this fix is totally correct.
> 
> diff --git a/useful_functions.c b/useful_functions.c

As said, post a patch that applies via git-format-patch for review.

Thanks.

> index 133ae2f..a8dfcbc 100644
> --- a/useful_functions.c
> +++ b/useful_functions.c
> @@ -364,8 +364,12 @@ static struct in6_addr *parse_ip6_mask(char *mask)
>         if (bits != 0) {
>                 char *p = (char *)&maskaddr;
>                 memset(p, 0xff, bits / 8);
> -               memset(p + (bits / 8) + 1, 0, (128 - bits) / 8);
> -               p[bits / 8] = 0xff << (8 - (bits & 7));
> +               if (bits & 7) {
> +                       memset(p + (bits / 8) + 1, 0, (128 - bits) / 8);
> +                       p[bits / 8] = 0xff << (8 - (bits & 7));
> +               } else {
> +                       memset(p + (bits / 8), 0, (128 - bits) / 8);
> +               }
>                 return &maskaddr;
>         }
> 

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Global buffer overflow in parse_ip6_mask()
  2025-12-18 14:44 ` Phil Sutter
@ 2026-02-11 13:08   ` Florian Westphal
  2026-02-11 19:43     ` Phil Sutter
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Westphal @ 2026-02-11 13:08 UTC (permalink / raw)
  To: Phil Sutter, Ilia Kashintsev, netfilter-devel

Phil Sutter <phil@nwl.cc> wrote:
> The reason why the second memset() call may mis-behave is the broken
> div-round-up in there: It does (bits / 8) + 1 when it should do
> (bits + 7) / 8 instead. Fixed that, only the p[bits / 8] field access
> needs to remain conditional:
> 
> @@ -364,8 +364,9 @@ static struct in6_addr *parse_ip6_mask(char *mask)
>         if (bits != 0) {
>                 char *p = (char *)&maskaddr;
>                 memset(p, 0xff, bits / 8);
> -               memset(p + (bits / 8) + 1, 0, (128 - bits) / 8);
> -               p[bits / 8] = 0xff << (8 - (bits & 7));
> +               memset(p + (bits + 7) / 8, 0, (128 - bits) / 8);
> +               if (bits & 7)
> +                       p[bits / 8] = 0xff << (8 - (bits & 7));
>                 return &maskaddr;
>         }

Phil, would you mind formally submitting this as fix?

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: Global buffer overflow in parse_ip6_mask()
  2026-02-11 13:08   ` Florian Westphal
@ 2026-02-11 19:43     ` Phil Sutter
  0 siblings, 0 replies; 5+ messages in thread
From: Phil Sutter @ 2026-02-11 19:43 UTC (permalink / raw)
  To: Florian Westphal; +Cc: Ilia Kashintsev, netfilter-devel

On Wed, Feb 11, 2026 at 02:08:42PM +0100, Florian Westphal wrote:
> Phil Sutter <phil@nwl.cc> wrote:
> > The reason why the second memset() call may mis-behave is the broken
> > div-round-up in there: It does (bits / 8) + 1 when it should do
> > (bits + 7) / 8 instead. Fixed that, only the p[bits / 8] field access
> > needs to remain conditional:
> > 
> > @@ -364,8 +364,9 @@ static struct in6_addr *parse_ip6_mask(char *mask)
> >         if (bits != 0) {
> >                 char *p = (char *)&maskaddr;
> >                 memset(p, 0xff, bits / 8);
> > -               memset(p + (bits / 8) + 1, 0, (128 - bits) / 8);
> > -               p[bits / 8] = 0xff << (8 - (bits & 7));
> > +               memset(p + (bits + 7) / 8, 0, (128 - bits) / 8);
> > +               if (bits & 7)
> > +                       p[bits / 8] = 0xff << (8 - (bits & 7));
> >                 return &maskaddr;
> >         }
> 
> Phil, would you mind formally submitting this as fix?

DONE, thanks for the reminder.

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2026-02-11 19:43 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-12-18 13:15 Global buffer overflow in parse_ip6_mask() Ilia Kashintsev
2025-12-18 14:44 ` Phil Sutter
2026-02-11 13:08   ` Florian Westphal
2026-02-11 19:43     ` Phil Sutter
2025-12-20 21:11 ` 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