lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <20230509212125.15880-10-stephen@networkplumber.org>
Date: Tue,  9 May 2023 14:21:23 -0700
From: Stephen Hemminger <stephen@...workplumber.org>
To: netdev@...r.kernel.org
Cc: Stephen Hemminger <stephen@...workplumber.org>
Subject: [PATCH iproute2 09/11] nstat: fix potential NULL deref

Reported as:
    CC       nstat
nstat.c: In function ‘load_ugly_table’:
nstat.c:205:24: warning: dereference of NULL ‘p’ [CWE-476] [-Wanalyzer-null-dereference]
  205 |                 while (*p) {
      |                        ^~
  ‘main’: events 1-14
    |
    |  575 | int main(int argc, char *argv[])
    |      |     ^~~~
    |      |     |
    |      |     (1) entry to ‘main’
    |......
    |  635 |         if (scan_interval > 0) {
    |      |            ~
    |      |            |
    |      |            (2) following ‘true’ branch...
    |  636 |                 if (time_constant == 0)
    |      |                     ~~~~~~~~~~~~~~~~~~
    |      |                                   |
    |      |                                   (3) ...to here
    |......
    |  640 |                 if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
    |      |                    ~      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                    |      |
    |      |                    |      (4) when ‘socket’ succeeds
    |      |                    (5) following ‘false’ branch (when ‘fd >= 0’)...
    |......
    |  644 |                 if (bind(fd, (struct sockaddr *)&sun, 2+1+strlen(sun.sun_path+1)) < 0) {
    |      |                    ~                                      ~~~~~~~~~~~~~~~~~~~~~~
    |      |                    |                                      |
    |      |                    (7) following ‘false’ branch...        (6) ...to here
    |......
    |  648 |                 if (listen(fd, 5) < 0) {
    |      |                    ~~~~~~~~~~~~~~
    |      |                    ||
    |      |                    |(8) ...to here
    |      |                    |(9) when ‘listen’ succeeds
    |      |                    (10) following ‘false’ branch...
    |......
    |  652 |                 if (daemon(0, 0)) {
    |      |                    ~~~~~~~~~~~~~
    |      |                    ||
    |      |                    |(11) ...to here
    |      |                    (12) following ‘false’ branch...
    |......
    |  656 |                 signal(SIGPIPE, SIG_IGN);
    |      |                 ~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                 |
    |      |                 (13) ...to here
    |  657 |                 signal(SIGCHLD, sigchild);
    |  658 |                 server_loop(fd);
    |      |                 ~~~~~~~~~~~~~~~
    |      |                 |
    |      |                 (14) calling ‘server_loop’ from ‘main’
    |
    +--> ‘server_loop’: events 15-16
           |
           |  472 | static void server_loop(int fd)
           |      |             ^~~~~~~~~~~
           |      |             |
           |      |             (15) entry to ‘server_loop’
           |......
           |  483 |         load_netstat();
           |      |         ~~~~~~~~~~~~~~
           |      |         |
           |      |         (16) calling ‘load_netstat’ from ‘server_loop’
           |
           +--> ‘load_netstat’: events 17-20
                  |
                  |  302 | static void load_netstat(void)
                  |      |             ^~~~~~~~~~~~
                  |      |             |
                  |      |             (17) entry to ‘load_netstat’
                  |......
                  |  306 |         if (fp) {
                  |      |            ~
                  |      |            |
                  |      |            (18) following ‘true’ branch (when ‘fp’ is non-NULL)...
                  |  307 |                 load_ugly_table(fp);
                  |      |                 ~~~~~~~~~~~~~~~~~~~
                  |      |                 |
                  |      |                 (19) ...to here
                  |      |                 (20) calling ‘load_ugly_table’ from ‘load_netstat’
                  |
                  +--> ‘load_ugly_table’: events 21-26
                         |
                         |  178 | static void load_ugly_table(FILE *fp)
                         |      |             ^~~~~~~~~~~~~~~
                         |      |             |
                         |      |             (21) entry to ‘load_ugly_table’
                         |  179 | {
                         |  180 |         char *buf = NULL;
                         |      |               ~~~
                         |      |               |
                         |      |               (22) ‘buf’ is NULL
                         |......
                         |  186 |         while ((nread = getline(&buf, &buflen, fp)) != -1) {
                         |      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                         |      |                                                     |
                         |      |                                                     (23) following ‘true’ branch...
                         |......
                         |  192 |                 p = strchr(buf, ':');
                         |      |                     ~~~~~~~~~~~~~~~~
                         |      |                     |
                         |      |                     (24) ...to here
                         |      |                     (25) when ‘strchr’ returns non-NULL
                         |  193 |                 if (!p) {
                         |      |                    ~
                         |      |                    |
                         |      |                    (26) following ‘false’ branch (when ‘p’ is non-NULL)...
                         |
                       ‘load_ugly_table’: event 27
                         |
                         |cc1:
                         | (27): ...to here
                         |
                       ‘load_ugly_table’: events 28-40
                         |
                         |  205 |                 while (*p) {
                         |      |                        ^~
                         |      |                        |
                         |      |                        (28) following ‘true’ branch...
                         |      |                        (40) dereference of NULL ‘p’
                         |......
                         |  208 |                         if ((next = strchr(p, ' ')) != NULL)
                         |      |                            ~        ~~~~~~~~~~~~~~
                         |      |                            |        |
                         |      |                            |        (29) ...to here
                         |      |                            |        (30) when ‘strchr’ returns NULL
                         |      |                            (31) following ‘false’ branch (when ‘next’ is NULL)...
                         |  209 |                                 *next++ = 0;
                         |  210 |                         else if ((next = strchr(p, '\n')) != NULL)
                         |      |                                 ~        ~~~~~~~~~~~~~~~
                         |      |                                 |        |
                         |      |                                 |        (32) ...to here
                         |      |                                 |        (33) when ‘strchr’ returns NULL
                         |      |                                 (34) following ‘false’ branch (when ‘next’ is NULL)...
                         |  211 |                                 *next++ = 0;
                         |  212 |                         if (off < sizeof(idbuf)) {
                         |      |                            ~~~~~~~~~~~~~~~~~~~~
                         |      |                            |    |
                         |      |                            |    (35) ...to here
                         |      |                            (36) following ‘false’ branch...
                         |......
                         |  216 |                         n = malloc(sizeof(*n));
                         |      |                             ~~~~~~~~~~~~~~~~~~
                         |      |                             |
                         |      |                             (37) ...to here
                         |  217 |                         if (!n) {
                         |      |                            ~
                         |      |                            |
                         |      |                            (38) following ‘false’ branch (when ‘n’ is non-NULL)...
                         |......
                         |  221 |                         n->id = strdup(idbuf);
                         |      |                                 ~~~~~~~~~~~~~
                         |      |                                 |
                         |      |                                 (39) ...to here
                         |
nstat.c:254:35: warning: dereference of NULL ‘n’ [CWE-476] [-Wanalyzer-null-dereference]
  254 |                                 n = n->next;
      |                                 ~~^~~~~~~~~
  ‘main’: events 1-14
    |
    |  575 | int main(int argc, char *argv[])
    |      |     ^~~~
    |      |     |
    |      |     (1) entry to ‘main’
    |......
    |  635 |         if (scan_interval > 0) {
    |      |            ~
    |      |            |
    |      |            (2) following ‘true’ branch...
    |  636 |                 if (time_constant == 0)
    |      |                     ~~~~~~~~~~~~~~~~~~
    |      |                                   |
    |      |                                   (3) ...to here
    |......
    |  640 |                 if ((fd = socket(AF_UNIX, SOCK_STREAM, 0)) < 0) {
    |      |                    ~      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                    |      |
    |      |                    |      (4) when ‘socket’ succeeds
    |      |                    (5) following ‘false’ branch (when ‘fd >= 0’)...
    |......
    |  644 |                 if (bind(fd, (struct sockaddr *)&sun, 2+1+strlen(sun.sun_path+1)) < 0) {
    |      |                    ~                                      ~~~~~~~~~~~~~~~~~~~~~~
    |      |                    |                                      |
    |      |                    (7) following ‘false’ branch...        (6) ...to here
    |......
    |  648 |                 if (listen(fd, 5) < 0) {
    |      |                    ~~~~~~~~~~~~~~
    |      |                    ||
    |      |                    |(8) ...to here
    |      |                    |(9) when ‘listen’ succeeds
    |      |                    (10) following ‘false’ branch...
    |......
    |  652 |                 if (daemon(0, 0)) {
    |      |                    ~~~~~~~~~~~~~
    |      |                    ||
    |      |                    |(11) ...to here
    |      |                    (12) following ‘false’ branch...
    |......
    |  656 |                 signal(SIGPIPE, SIG_IGN);
    |      |                 ~~~~~~~~~~~~~~~~~~~~~~~~
    |      |                 |
    |      |                 (13) ...to here
    |  657 |                 signal(SIGCHLD, sigchild);
    |  658 |                 server_loop(fd);
    |      |                 ~~~~~~~~~~~~~~~
    |      |                 |
    |      |                 (14) calling ‘server_loop’ from ‘main’
    |
    +--> ‘server_loop’: events 15-16
           |
           |  472 | static void server_loop(int fd)
           |      |             ^~~~~~~~~~~
           |      |             |
           |      |             (15) entry to ‘server_loop’
           |......
           |  483 |         load_netstat();
           |      |         ~~~~~~~~~~~~~~
           |      |         |
           |      |         (16) calling ‘load_netstat’ from ‘server_loop’
           |
           +--> ‘load_netstat’: events 17-20
                  |
                  |  302 | static void load_netstat(void)
                  |      |             ^~~~~~~~~~~~
                  |      |             |
                  |      |             (17) entry to ‘load_netstat’
                  |......
                  |  306 |         if (fp) {
                  |      |            ~
                  |      |            |
                  |      |            (18) following ‘true’ branch (when ‘fp’ is non-NULL)...
                  |  307 |                 load_ugly_table(fp);
                  |      |                 ~~~~~~~~~~~~~~~~~~~
                  |      |                 |
                  |      |                 (19) ...to here
                  |      |                 (20) calling ‘load_ugly_table’ from ‘load_netstat’
                  |
                  +--> ‘load_ugly_table’: events 21-25
                         |
                         |  178 | static void load_ugly_table(FILE *fp)
                         |      |             ^~~~~~~~~~~~~~~
                         |      |             |
                         |      |             (21) entry to ‘load_ugly_table’
                         |......
                         |  186 |         while ((nread = getline(&buf, &buflen, fp)) != -1) {
                         |      |                ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                         |      |                                                     |
                         |      |                                                     (22) following ‘true’ branch...
                         |......
                         |  192 |                 p = strchr(buf, ':');
                         |      |                     ~~~~~~~~~~~~~~~~
                         |      |                     |
                         |      |                     (23) ...to here
                         |      |                     (24) when ‘strchr’ returns non-NULL
                         |  193 |                 if (!p) {
                         |      |                    ~
                         |      |                    |
                         |      |                    (25) following ‘false’ branch (when ‘p’ is non-NULL)...
                         |
                       ‘load_ugly_table’: event 26
                         |
                         |cc1:
                         | (26): ...to here
                         |
                       ‘load_ugly_table’: events 27-28
                         |
                         |  205 |                 while (*p) {
                         |      |                        ^
                         |      |                        |
                         |      |                        (27) following ‘false’ branch...
                         |......
                         |  228 |                 nread = getline(&buf, &buflen, fp);
                         |      |                         ~
                         |      |                         |
                         |      |                         (28) inlined call to ‘getline’ from ‘load_ugly_table’
                         |
                         +--> ‘getline’: event 29
                                |
                                |/usr/include/bits/stdio.h:120:10:
                                |  120 |   return __getdelim (__lineptr, __n, '\n', __stream);
                                |      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
                                |      |          |
                                |      |          (29) ...to here
                                |
                         <------+
                         |
                       ‘load_ugly_table’: events 30-36
                         |
                         |nstat.c:229:20:
                         |  229 |                 if (nread == -1) {
                         |      |                    ^
                         |      |                    |
                         |      |                    (30) following ‘false’ branch...
                         |......
                         |  234 |                 count2 = count_spaces(buf);
                         |      |                          ~~~~~~~~~~~~~~~~~
                         |      |                          |
                         |      |                          (31) ...to here
                         |......
                         |  239 |                         if (!p) {
                         |      |                            ~
                         |      |                            |
                         |      |                            (32) following ‘false’ branch (when ‘p’ is non-NULL)...
                         |......
                         |  244 |                         *p = 0;
                         |      |                         ~~~~~~
                         |      |                            |
                         |      |                            (33) ...to here
                         |  245 |                         if (sscanf(p+1, "%llu", &n->val) != 1) {
                         |      |                            ~
                         |      |                            |
                         |      |                            (34) following ‘false’ branch...
                         |......
                         |  251 |                         if (skip)
                         |      |                            ~
                         |      |                            |
                         |      |                            (35) ...to here
                         |......
                         |  254 |                                 n = n->next;
                         |      |                                 ~~~~~~~~~~~
                         |      |                                   |
                         |      |                                   (36) dereference of NULL ‘n’
                         |

Signed-off-by: Stephen Hemminger <stephen@...workplumber.org>
---
 misc/nstat.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/misc/nstat.c b/misc/nstat.c
index 0ab92ecbeb47..2c10feaa3adf 100644
--- a/misc/nstat.c
+++ b/misc/nstat.c
@@ -219,9 +219,15 @@ static void load_ugly_table(FILE *fp)
 				exit(-1);
 			}
 			n->id = strdup(idbuf);
+			if (n->id == NULL) {
+				perror("nstat: strdup");
+				exit(-1);
+			}
 			n->rate = 0;
 			n->next = db;
 			db = n;
+			if (next == NULL)
+				break;
 			p = next;
 		}
 		n = db;
-- 
2.39.2


Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ