318 lines
14 KiB
Diff
318 lines
14 KiB
Diff
|
|
From 8cda7a24a971170b833009f392579cfea87711bf Mon Sep 17 00:00:00 2001
|
|||
|
|
From: Stephen Hemminger <stephen@networkplumber.org>
|
|||
|
|
Date: Mon, 8 May 2023 19:02:20 -0700
|
|||
|
|
Subject: [PATCH] ipmaddr: fix dereference of NULL on malloc() failure
|
|||
|
|
MIME-Version: 1.0
|
|||
|
|
Content-Type: text/plain; charset=UTF-8
|
|||
|
|
Content-Transfer-Encoding: 8bit
|
|||
|
|
|
|||
|
|
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>
|
|||
|
|
---
|
|||
|
|
ip/ipmaddr.c | 9 ++++++++-
|
|||
|
|
1 file changed, 8 insertions(+), 1 deletion(-)
|
|||
|
|
|
|||
|
|
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);
|
|||
|
|
--
|
|||
|
|
2.27.0
|
|||
|
|
|