ulogd: out of bounds array access in ulogd_filter_HWHDR

ulogd: out of bounds array access in ulogd_filter_HWHDR

[James Dingwall]

[-- Attachment #1: Type: text/plain, Size: 1795 bytes --]

Hi,

I've been given an account in the bugzilla but on submitting:

Forbidden

You don't have permission to access this resource.


Here's what I'm trying to report...

Thanks,
James



This report relates to https://bugs.launchpad.net/ubuntu/+source/ulogd2/+bug/2080677.

# apt-cache policy ulogd2
ulogd2:
  Installed: 2.0.8-2build1
  Candidate: 2.0.8-2build1
  Version table:
 *** 2.0.8-2build1 500
        500 http://gb.archive.ubuntu.com/ubuntu noble/universe amd64 Packages
        100 /var/lib/dpkg/status

# lsb_release -a
No LSB modules are available.
Distributor ID: Ubuntu
Description: Ubuntu 24.04.1 LTS
Release: 24.04
Codename: noble

It seems that there is an out of bounds array access in ulogd_filter_HWHDR.c
which leads to ulogd2 being terminated with SIGABRT and the following message
when it is compiled with -D_FORTIFY_SOURCE=3:

*** buffer overflow detected ***

The hwac_str array is defined as:

  static char hwmac_str[MAX_KEY - START_KEY][HWADDR_LENGTH];

Which translates to:

  static char hwmac_str[4 - 2][128];

i.e. an array of two elements, valid indexes 0, 1.

Adding a debug print statement in the parse_mac2str function:

  fprintf(stderr, "using hwmac_str index %d\n", okey - START_KEY);

will result in the following message:  

  using hwmac_str index 2

So the for loop attempts to format the mac address in to an invalid index in
hwmac_str.

As a simple test I made the definition of hwmac_str an array of 3 elements
which prevented the crash.  I don't know if it is correct to simply make
the array longer or if the bug is actually in the value of 'okey' passed to
the function.  However based on the final return in interp_mac2str I think
the array definition is too short.  The attached patch allows ulog2 to
run after rebuilding with dpkg-buildpackage.

[-- Attachment #2: ulogd_filter_HWHDR-hwmac_str.patch --]
[-- Type: text/x-diff, Size: 393 bytes --]

--- filter/ulogd_filter_HWHDR.c.orig	2025-01-13 09:25:18.937977335 +0000
+++ filter/ulogd_filter_HWHDR.c	2025-01-13 09:25:51.337824820 +0000
@@ -109,7 +109,7 @@
 	},
 };
 
-static char hwmac_str[MAX_KEY - START_KEY][HWADDR_LENGTH];
+static char hwmac_str[(MAX_KEY + 1) - START_KEY][HWADDR_LENGTH];
 
 static int parse_mac2str(struct ulogd_key *ret, unsigned char *mac,
 			 int okey, int len)

Re: ulogd: out of bounds array access in ulogd_filter_HWHDR

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 2476 bytes --]

On 2025-01-13, at 10:33:10 +0000, James Dingwall wrote:
> This report relates to https://bugs.launchpad.net/ubuntu/+source/ulogd2/+bug/2080677.
> 
> # apt-cache policy ulogd2
> ulogd2:
>   Installed: 2.0.8-2build1
>   Candidate: 2.0.8-2build1
>   Version table:
>  *** 2.0.8-2build1 500
>         500 http://gb.archive.ubuntu.com/ubuntu noble/universe amd64 Packages
>         100 /var/lib/dpkg/status
> 
> # lsb_release -a
> No LSB modules are available.
> Distributor ID: Ubuntu
> Description: Ubuntu 24.04.1 LTS
> Release: 24.04
> Codename: noble
> 
> It seems that there is an out of bounds array access in ulogd_filter_HWHDR.c
> which leads to ulogd2 being terminated with SIGABRT and the following message
> when it is compiled with -D_FORTIFY_SOURCE=3:
> 
> *** buffer overflow detected ***
> 
> The hwac_str array is defined as:
> 
>   static char hwmac_str[MAX_KEY - START_KEY][HWADDR_LENGTH];
> 
> Which translates to:
> 
>   static char hwmac_str[4 - 2][128];
> 
> i.e. an array of two elements, valid indexes 0, 1.
> 
> Adding a debug print statement in the parse_mac2str function:
> 
>   fprintf(stderr, "using hwmac_str index %d\n", okey - START_KEY);
> 
> will result in the following message:  
> 
>   using hwmac_str index 2
> 
> So the for loop attempts to format the mac address in to an invalid index in
> hwmac_str.
> 
> As a simple test I made the definition of hwmac_str an array of 3 elements
> which prevented the crash.  I don't know if it is correct to simply make
> the array longer or if the bug is actually in the value of 'okey' passed to
> the function.  However based on the final return in interp_mac2str I think
> the array definition is too short.  The attached patch allows ulog2 to
> run after rebuilding with dpkg-buildpackage.

> --- filter/ulogd_filter_HWHDR.c.orig	2025-01-13 09:25:18.937977335 +0000
> +++ filter/ulogd_filter_HWHDR.c	2025-01-13 09:25:51.337824820 +0000
> @@ -109,7 +109,7 @@
>  	},
>  };
>  
> -static char hwmac_str[MAX_KEY - START_KEY][HWADDR_LENGTH];
> +static char hwmac_str[(MAX_KEY + 1) - START_KEY][HWADDR_LENGTH];
>  
>  static int parse_mac2str(struct ulogd_key *ret, unsigned char *mac,
>  			 int okey, int len)

This was fixed a couple of years ago:

  https://git.netfilter.org/ulogd2/commit/?id=49f6def6fcbaf01f395fbe00543a9ab2c4bb106e

and the fix should have made it into the Debian & Ubuntu packages.  I
will investigate.

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 931 bytes --]

Re: ulogd: out of bounds array access in ulogd_filter_HWHDR

[Pablo Neira Ayuso]

On Mon, Jan 13, 2025 at 11:12:01AM +0000, Jeremy Sowden wrote:
> On 2025-01-13, at 10:33:10 +0000, James Dingwall wrote:
[...]
> > --- filter/ulogd_filter_HWHDR.c.orig	2025-01-13 09:25:18.937977335 +0000
> > +++ filter/ulogd_filter_HWHDR.c	2025-01-13 09:25:51.337824820 +0000
> > @@ -109,7 +109,7 @@
> >  	},
> >  };
> >  
> > -static char hwmac_str[MAX_KEY - START_KEY][HWADDR_LENGTH];
> > +static char hwmac_str[(MAX_KEY + 1) - START_KEY][HWADDR_LENGTH];
> >  
> >  static int parse_mac2str(struct ulogd_key *ret, unsigned char *mac,
> >  			 int okey, int len)
> 
> This was fixed a couple of years ago:
> 
>   https://git.netfilter.org/ulogd2/commit/?id=49f6def6fcbaf01f395fbe00543a9ab2c4bb106e
> 
> and the fix should have made it into the Debian & Ubuntu packages.  I
> will investigate.

I am going to launch a new release to help this propagate to distros.

Re: ulogd: out of bounds array access in ulogd_filter_HWHDR

[Jeremy Sowden]

[-- Attachment #1: Type: text/plain, Size: 1050 bytes --]

On 2025-01-13, at 13:30:01 +0100, Pablo Neira Ayuso wrote:
> On Mon, Jan 13, 2025 at 11:12:01AM +0000, Jeremy Sowden wrote:
> > On 2025-01-13, at 10:33:10 +0000, James Dingwall wrote:
> [...]
> > > --- filter/ulogd_filter_HWHDR.c.orig	2025-01-13 09:25:18.937977335 +0000
> > > +++ filter/ulogd_filter_HWHDR.c	2025-01-13 09:25:51.337824820 +0000
> > > @@ -109,7 +109,7 @@
> > >  	},
> > >  };
> > >  
> > > -static char hwmac_str[MAX_KEY - START_KEY][HWADDR_LENGTH];
> > > +static char hwmac_str[(MAX_KEY + 1) - START_KEY][HWADDR_LENGTH];
> > >  
> > >  static int parse_mac2str(struct ulogd_key *ret, unsigned char *mac,
> > >  			 int okey, int len)
> > 
> > This was fixed a couple of years ago:
> > 
> >   https://git.netfilter.org/ulogd2/commit/?id=49f6def6fcbaf01f395fbe00543a9ab2c4bb106e
> > 
> > and the fix should have made it into the Debian & Ubuntu packages.  I
> > will investigate.
> 
> I am going to launch a new release to help this propagate to distros.

Thanks, Pablo.  I was going to request one. :)

J.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 931 bytes --]

bugzilla forbiden issue [was Re: ulogd: out of bounds array access in ulogd_filter_HWHDR]

[Pablo Neira Ayuso]

Hi,

On Mon, Jan 13, 2025 at 10:33:10AM +0000, James Dingwall wrote:
> Hi,
> 
> I've been given an account in the bugzilla but on submitting:
> 
> Forbidden
> 
> You don't have permission to access this resource.

This is an issue on the server side, would you please provide some
information privately to debug it?

Thanks.

Re: bugzilla forbiden issue [was Re: ulogd: out of bounds array access in ulogd_filter_HWHDR]

[Florian Westphal]

Pablo Neira Ayuso <pablo@netfilter.org> wrote:
> On Mon, Jan 13, 2025 at 10:33:10AM +0000, James Dingwall wrote:
> > Hi,
> > 
> > I've been given an account in the bugzilla but on submitting:
> > 
> > Forbidden
> > 
> > You don't have permission to access this resource.
> 
> This is an issue on the server side, would you please provide some
> information privately to debug it?

I can't log in either, just tried.

Its enough to enter my username/password on
https://bugzilla.netfilter.org/index.cgi?GoAheadAndLogIn=1