[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20250225101954.3fdb009f@hermes.local>
Date: Tue, 25 Feb 2025 10:19:54 -0800
From: Stephen Hemminger <stephen@...workplumber.org>
To: Ziao Li <leeziao0331@...il.com>
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH iproute2] NULL Pointer Dereference vulnerability and
patch
On Wed, 19 Feb 2025 17:11:51 +0800
Ziao Li <leeziao0331@...il.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@...il.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.
Powered by blists - more mailing lists