Re: [PATCH iproute2] NULL Pointer Dereference vulnerability and patch

[Stephen Hemminger <stephen@networkplumber.org>]

On Wed, 19 Feb 2025 17:11:51 +0800
Ziao Li <leeziao0331@gmail.com> wrote:

> NULL Pointer Dereference vulnerability in iproute2.
> The vulnerability happens in load_ugly_table(), misc/nstat.c, in the
> latest version of iproute2 (41710ace5e8fadff354f3dba67bf27ed3a3c5ae7)
> How the vulnerability happens:
> 1. db is set to NULL at struct nstat_ent *db = NULL;
> 2. n is set to NULL at n = db;
> 3. NULL dereference of variable n happens at sscanf(p+1, "%llu", &n->val) != 1
> static void load_ugly_table(FILE *fp)
> {
>         char *buf = NULL;
>         size_t buflen = 0;
>         ssize_t nread;
>         struct nstat_ent *db = NULL;
>         struct nstat_ent *n;
> 
>         while ((nread = getline(&buf, &buflen, fp)) != -1) {
>                 char idbuf[4096];
>                 int  off;
>                 char *p;
>                 int count1, count2, skip = 0;
> 
>                 p = strchr(buf, ':');
>                 if (!p) {
>                         fprintf(stderr, "%s:%d: error parsing history file\n",
>                                 __FILE__, __LINE__);
>                         exit(-2);
>                 }
>                 count1 = count_spaces(buf);
>                 *p = 0;
>                 idbuf[0] = 0;
>                 strncat(idbuf, buf, sizeof(idbuf) - 1);
>                 off = p - buf;
>                 p += 2;
> 
>                 while (*p) {
>                     ......
>                 }
>                 n = db;
>                 nread = getline(&buf, &buflen, fp);
>                 if (nread == -1) {
>                         fprintf(stderr, "%s:%d: error parsing history file\n",
>                                 __FILE__, __LINE__);
>                         exit(-2);
>                 }
>                 count2 = count_spaces(buf);
>                 if (count2 > count1)
>                         skip = count2 - count1;
>                 do {
>                         p = strrchr(buf, ' ');
>                         if (!p) {
>                                 fprintf(stderr, "%s:%d: error parsing
> history file\n",
>                                         __FILE__, __LINE__);
>                                 exit(-2);
>                         }
>                         *p = 0;
>                         if (sscanf(p+1, "%llu", &n->val) != 1) {
>                                 fprintf(stderr, "%s:%d: error parsing
> history file\n",
>                                         __FILE__, __LINE__);
>                                 exit(-2);
>                         }
>                         /* Trick to skip "dummy" trailing ICMP MIB in 2.4 */
>                         if (skip)
>                                 skip--;
>                         else
>                                 n = n->next;
>                 } while (p > buf + off + 2);
>         }
>         free(buf);
>         ......
> }
> 
> ---
> Steps to reproduce:
> 1. Put attachment files file at misc/poc.c and misc/crash.txt
> 2. Compile poc.c file with:
> gcc -Wall -Wstrict-prototypes  -Wmissing-prototypes
> -Wmissing-declarations -Wold-style-definition -Wformat=2 -g -O0 -pipe
> -I../include -I../include/uapi -DRESOLVE_HOSTNAMES
> -DLIBDIR=\"/usr/lib\" -DCONF_USR_DIR=\"/usr/share/iproute2\"
> -DCONF_ETC_DIR=\"/etc/iproute2\" -DNETNS_RUN_DIR=\"/var/run/netns\"
> -DNETNS_ETC_DIR=\"/etc/netns\" -DARPDDIR=\"/var/lib/arpd\"
> -DCONF_COLOR=COLOR_OPT_NEVER -D_GNU_SOURCE -D_FILE_OFFSET_BITS=64
> -D_LARGEFILE_SOURCE -D_LARGEFILE64_SOURCE -DHAVE_SETNS
> -DHAVE_HANDLE_AT -DHAVE_SELINUX  -DHAVE_RPC -I/usr/include/tirpc
> -DHAVE_ELF  -DNEED_STRLCPY -DHAVE_LIBCAP  -DHAVE_SETNS
> -DHAVE_HANDLE_AT -DHAVE_SELINUX  -DHAVE_RPC -I/usr/include/tirpc
> -DHAVE_ELF  -DNEED_STRLCPY -DHAVE_LIBCAP -o poc poc.c -lselinux
> -ltirpc -lelf -lcap ../lib/libutil.a ../lib/libnetlink.a -lselinux
> -ltirpc -lelf -lcap -lm
> 3. Run the poc by
> $ ./poc crash.txt
> zsh: segmentation fault (core dumped)  ./poc crash.txt
> ---
> Patch for the vulnerability:
> 
> From 2f462d5adf071827285291d2ce13119e220681fd Mon Sep 17 00:00:00 2001
> From: lza <leeziao0331@gmail.com>
> Date: Wed, 19 Feb 2025 08:38:48 +0000
> Subject: [PATCH] Fix Null Dereference when no entries are specified
> 
> ---
>  misc/nstat.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/misc/nstat.c b/misc/nstat.c
> index fce3e9c1..b2e19bde 100644
> --- a/misc/nstat.c
> +++ b/misc/nstat.c
> @@ -218,6 +218,10 @@ static void load_ugly_table(FILE *fp)
>             p = next;
>         }
>         n = db;
> +       if (n == NULL) {
> +           fprintf(stderr, "Error: Invalid input – line has ':' but
> no entries. Add values after ':'.\n");
> +           exit(-2);
> +       }
>         nread = getline(&buf, &buflen, fp);
>         if (nread == -1) {
>             fprintf(stderr, "%s:%d: error parsing history file\n",


Thank you for finding and reporting a fix for a bug.
But this is not a vulnerability, just a bug.
And the patch is not formatted properly (line wrap) and
required Signed-off-by is missing.

Please fix and resubmit.