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:
signature.asc
Description: PGP signature
