ipmaddr: fix dereference of NULL on malloc() failure

Found by -fanalyzer. This is a bug since beginning of initial
versions of ip multicast support (pre git).

ipmaddr.c: In function ‘read_dev_mcast’:
ipmaddr.c:105:25: warning: dereference of possibly-NULL ‘ma’ [CWE-690] [-Wanalyzer-possible-null-dereference]
  105 |                         memcpy(ma, &m, sizeof(m));
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
  ‘do_multiaddr’: events 1-4
    |
    |  354 | int do_multiaddr(int argc, char **argv)
    |      |     ^~~~~~~~~~~~
    |      |     |
    |      |     (1) entry to ‘do_multiaddr’
    |  355 | {
    |  356 |         if (argc < 1)
    |      |            ~
    |      |            |
    |      |            (2) following ‘true’ branch (when ‘argc <= 0’)...
    |  357 |                 return multiaddr_list(0, NULL);
    |      |                        ~~~~~~~~~~~~~~~~~~~~~~~
    |      |                        |
    |      |                        (3) ...to here
    |      |                        (4) calling ‘multiaddr_list’ from ‘do_multiaddr’
    |
    +--> ‘multiaddr_list’: events 5-10
           |
           |  255 | static int multiaddr_list(int argc, char **argv)
           |      |            ^~~~~~~~~~~~~~
           |      |            |
           |      |            (5) entry to ‘multiaddr_list’
           |......
           |  262 |         while (argc > 0) {
           |      |                ~~~~~~~~
           |      |                     |
           |      |                     (6) following ‘false’ branch (when ‘argc <= 0’)...
           |......
           |  275 |         if (!filter.family || filter.family == AF_PACKET)
           |      |            ~ ~~~~~~~~~~~~~
           |      |            |       |
           |      |            |       (7) ...to here
           |      |            (8) following ‘true’ branch...
           |  276 |                 read_dev_mcast(&list);
           |      |                 ~~~~~~~~~~~~~~~~~~~~~
           |      |                 |
           |      |                 (9) ...to here
           |      |                 (10) calling ‘read_dev_mcast’ from ‘multiaddr_list’
           |
           +--> ‘read_dev_mcast’: events 11-12
                  |
                  |   82 | static void read_dev_mcast(struct ma_info **result_p)
                  |      |             ^~~~~~~~~~~~~~
                  |      |             |
                  |      |             (11) entry to ‘read_dev_mcast’
                  |......
                  |   87 |         if (!fp)
                  |      |            ~
                  |      |            |
                  |      |            (12) following ‘false’ branch (when ‘fp’ is non-NULL)...
                  |
                ‘read_dev_mcast’: event 13
                  |
                  |cc1:
                  | (13): ...to here
                  |
                ‘read_dev_mcast’: events 14-17
                  |
                  |   90 |         while (fgets(buf, sizeof(buf), fp)) {
                  |      |                ^~~~~
                  |      |                |
                  |      |                (14) following ‘true’ branch...
                  |   91 |                 char hexa[256];
                  |   92 |                 struct ma_info m = { .addr.family = AF_PACKET };
                  |      |                                ~
                  |      |                                |
                  |      |                                (15) ...to here
                  |......
                  |  103 |                         struct ma_info *ma = malloc(sizeof(m));
                  |      |                                              ~~~~~~~~~~~~~~~~~
                  |      |                                              |
                  |      |                                              (16) this call could return NULL
                  |  104 |
                  |  105 |                         memcpy(ma, &m, sizeof(m));
                  |      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |                         |
                  |      |                         (17) ‘ma’ could be NULL: unchecked value from (16)
                  |
ipmaddr.c: In function ‘read_igmp’:
ipmaddr.c:152:17: warning: dereference of possibly-NULL ‘ma’ [CWE-690] [-Wanalyzer-possible-null-dereference]
  152 |                 memcpy(ma, &m, sizeof(m));
      |                 ^~~~~~~~~~~~~~~~~~~~~~~~~
  ‘do_multiaddr’: events 1-4
    |
    |  354 | int do_multiaddr(int argc, char **argv)
    |      |     ^~~~~~~~~~~~
    |      |     |
    |      |     (1) entry to ‘do_multiaddr’
    |  355 | {
    |  356 |         if (argc < 1)
    |      |            ~
    |      |            |
    |      |            (2) following ‘true’ branch (when ‘argc <= 0’)...
    |  357 |                 return multiaddr_list(0, NULL);
    |      |                        ~~~~~~~~~~~~~~~~~~~~~~~
    |      |                        |
    |      |                        (3) ...to here
    |      |                        (4) calling ‘multiaddr_list’ from ‘do_multiaddr’
    |
    +--> ‘multiaddr_list’: events 5-10
           |
           |  255 | static int multiaddr_list(int argc, char **argv)
           |      |            ^~~~~~~~~~~~~~
           |      |            |
           |      |            (5) entry to ‘multiaddr_list’
           |......
           |  262 |         while (argc > 0) {
           |      |                ~~~~~~~~
           |      |                     |
           |      |                     (6) following ‘false’ branch (when ‘argc <= 0’)...
           |......
           |  275 |         if (!filter.family || filter.family == AF_PACKET)
           |      |              ~~~~~~~~~~~~~
           |      |                    |
           |      |                    (7) ...to here
           |  276 |                 read_dev_mcast(&list);
           |  277 |         if (!filter.family || filter.family == AF_INET)
           |      |            ~
           |      |            |
           |      |            (8) following ‘true’ branch...
           |  278 |                 read_igmp(&list);
           |      |                 ~~~~~~~~~~~~~~~~
           |      |                 |
           |      |                 (9) ...to here
           |      |                 (10) calling ‘read_igmp’ from ‘multiaddr_list’
           |
           +--> ‘read_igmp’: events 11-14
                  |
                  |  116 | static void read_igmp(struct ma_info **result_p)
                  |      |             ^~~~~~~~~
                  |      |             |
                  |      |             (11) entry to ‘read_igmp’
                  |......
                  |  126 |         if (!fp)
                  |      |            ~
                  |      |            |
                  |      |            (12) following ‘false’ branch (when ‘fp’ is non-NULL)...
                  |  127 |                 return;
                  |  128 |         if (!fgets(buf, sizeof(buf), fp)) {
                  |      |            ~ ~~~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |            | |
                  |      |            | (13) ...to here
                  |      |            (14) following ‘false’ branch...
                  |
                ‘read_igmp’: event 15
                  |
                  |cc1:
                  | (15): ...to here
                  |
                ‘read_igmp’: events 16-19
                  |
                  |  133 |         while (fgets(buf, sizeof(buf), fp)) {
                  |      |                ^~~~~
                  |      |                |
                  |      |                (16) following ‘true’ branch...
                  |......
                  |  136 |                 if (buf[0] != '\t') {
                  |      |                     ~~~~~~
                  |      |                        |
                  |      |                        (17) ...to here
                  |......
                  |  151 |                 ma = malloc(sizeof(m));
                  |      |                      ~~~~~~~~~~~~~~~~~
                  |      |                      |
                  |      |                      (18) this call could return NULL
                  |  152 |                 memcpy(ma, &m, sizeof(m));
                  |      |                 ~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |                 |
                  |      |                 (19) ‘ma’ could be NULL: unchecked value from (18)
                  |
ipmaddr.c: In function ‘read_igmp6’:
ipmaddr.c:181:25: warning: dereference of possibly-NULL ‘ma’ [CWE-690] [-Wanalyzer-possible-null-dereference]
  181 |                         memcpy(ma, &m, sizeof(m));
      |                         ^~~~~~~~~~~~~~~~~~~~~~~~~
  ‘do_multiaddr’: events 1-4
    |
    |  354 | int do_multiaddr(int argc, char **argv)
    |      |     ^~~~~~~~~~~~
    |      |     |
    |      |     (1) entry to ‘do_multiaddr’
    |  355 | {
    |  356 |         if (argc < 1)
    |      |            ~
    |      |            |
    |      |            (2) following ‘true’ branch (when ‘argc <= 0’)...
    |  357 |                 return multiaddr_list(0, NULL);
    |      |                        ~~~~~~~~~~~~~~~~~~~~~~~
    |      |                        |
    |      |                        (3) ...to here
    |      |                        (4) calling ‘multiaddr_list’ from ‘do_multiaddr’
    |
    +--> ‘multiaddr_list’: events 5-10
           |
           |  255 | static int multiaddr_list(int argc, char **argv)
           |      |            ^~~~~~~~~~~~~~
           |      |            |
           |      |            (5) entry to ‘multiaddr_list’
           |......
           |  262 |         while (argc > 0) {
           |      |                ~~~~~~~~
           |      |                     |
           |      |                     (6) following ‘false’ branch (when ‘argc <= 0’)...
           |......
           |  275 |         if (!filter.family || filter.family == AF_PACKET)
           |      |              ~~~~~~~~~~~~~
           |      |                    |
           |      |                    (7) ...to here
           |......
           |  279 |         if (!filter.family || filter.family == AF_INET6)
           |      |            ~
           |      |            |
           |      |            (8) following ‘true’ branch...
           |  280 |                 read_igmp6(&list);
           |      |                 ~~~~~~~~~~~~~~~~~
           |      |                 |
           |      |                 (9) ...to here
           |      |                 (10) calling ‘read_igmp6’ from ‘multiaddr_list’
           |
           +--> ‘read_igmp6’: events 11-12
                  |
                  |  159 | static void read_igmp6(struct ma_info **result_p)
                  |      |             ^~~~~~~~~~
                  |      |             |
                  |      |             (11) entry to ‘read_igmp6’
                  |......
                  |  164 |         if (!fp)
                  |      |            ~
                  |      |            |
                  |      |            (12) following ‘false’ branch (when ‘fp’ is non-NULL)...
                  |
                ‘read_igmp6’: event 13
                  |
                  |cc1:
                  | (13): ...to here
                  |
                ‘read_igmp6’: events 14-17
                  |
                  |  167 |         while (fgets(buf, sizeof(buf), fp)) {
                  |      |                ^~~~~
                  |      |                |
                  |      |                (14) following ‘true’ branch...
                  |  168 |                 char hexa[256];
                  |  169 |                 struct ma_info m = { .addr.family = AF_INET6 };
                  |      |                                ~
                  |      |                                |
                  |      |                                (15) ...to here
                  |......
                  |  179 |                         struct ma_info *ma = malloc(sizeof(m));
                  |      |                                              ~~~~~~~~~~~~~~~~~
                  |      |                                              |
                  |      |                                              (16) this call could return NULL
                  |  180 |
                  |  181 |                         memcpy(ma, &m, sizeof(m));
                  |      |                         ~~~~~~~~~~~~~~~~~~~~~~~~~
                  |      |                         |
                  |      |                         (17) ‘ma’ could be NULL: unchecked value from (16)
                  |

Signed-off-by: Stephen Hemminger <stephen@networkplumber.org>

1 files changed, 8 insertions, 1 deletions

diff --git a/ip/ipmaddr.c b/ip/ipmaddr.c
index f8d6b992..a8ef20ec 100644
--- a/ip/ipmaddr.c
+++ b/ip/ipmaddr.c
@@ -102,6 +102,8 @@ static void read_dev_mcast(struct ma_info **result_p)
 		if (len >= 0) {
 			struct ma_info *ma = malloc(sizeof(m));
 
+			if (ma == NULL)
+				break;
 			memcpy(ma, &m, sizeof(m));
 			ma->addr.bytelen = len;
 			ma->addr.bitlen = len<<3;
@@ -149,6 +151,9 @@ static void read_igmp(struct ma_info **result_p)
 		sscanf(buf, "%08x%d", (__u32 *)&m.addr.data, &m.users);
 
 		ma = malloc(sizeof(m));
+		if (ma == NULL)
+			break;
+			
 		memcpy(ma, &m, sizeof(m));
 		maddr_ins(result_p, ma);
 	}
@@ -178,8 +183,10 @@ static void read_igmp6(struct ma_info **result_p)
 		if (len >= 0) {
 			struct ma_info *ma = malloc(sizeof(m));
 
-			memcpy(ma, &m, sizeof(m));
+			if (ma == NULL)
+				break;
 
+			memcpy(ma, &m, sizeof(m));
 			ma->addr.bytelen = len;
 			ma->addr.bitlen = len<<3;
 			maddr_ins(result_p, ma);