rpms/bluez
7
0
Fork 0
Code Issues Pull Requests Releases Activity

Fix coverity issues

Browse Source 
Related: Jira:RHEL-34536
This commit is contained in:
Bastien Nocera 2024-08-05 17:15:39 +02:00
parent b609205c43
commit 827e91509a
6 changed files with 4990 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

81
0001-main-Simplify-parse_config_string.patch Normal file
View File

@ -0,0 +1,81 @@
From 3cf5bb59c3f82e1fcc8703e6bab956284f2c4566 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Fri, 10 May 2024 13:47:29 +0200
Subject: [PATCH] main: Simplify parse_config_string()
The memory management done by parse_config_string() was quite
complicated, as it expected to be able to free the value in the return
variable if it was already allocated.
That particular behaviour was only used for a single variable which was
set to its default value during startup and might be overwritten after
this function call.
Use an intermediate variable to check whether we need to free
btd_opts.name and simplify parse_config_string().
Error: RESOURCE_LEAK (CWE-772): [#def39] [important]
bluez-5.75/src/main.c:425:2: alloc_fn: Storage is returned from allocation function "g_key_file_get_string".
bluez-5.75/src/main.c:425:2: var_assign: Assigning: "tmp" = storage returned from "g_key_file_get_string(config, group, key, &err)".
bluez-5.75/src/main.c:433:2: noescape: Assuming resource "tmp" is not freed or pointed-to as ellipsis argument to "btd_debug".
bluez-5.75/src/main.c:440:2: leaked_storage: Variable "tmp" going out of scope leaks the storage it points to.
438| }
439|
440|-> return true;
441| }
442|
---
src/main.c | 22 +++++++++++++---------
1 file changed, 13 insertions(+), 9 deletions(-)
diff --git a/src/main.c b/src/main.c
index 62453bffaf57..178611e11ddd 100644
--- a/src/main.c
+++ b/src/main.c
@@ -420,9 +420,13 @@ static bool parse_config_string(GKeyFile *config, const char *group,
const char *key, char **val)
{
GError *err = NULL;
- char *tmp;
- tmp = g_key_file_get_string(config, group, key, &err);
+ if (val != NULL) {
+ warn("%s passed a NULL value", __func__);
+ return false;
+ }
+
+ *val = g_key_file_get_string(config, group, key, &err);
if (err) {
if (err->code != G_KEY_FILE_ERROR_KEY_NOT_FOUND)
DBG("%s", err->message);
@@ -430,12 +434,7 @@ static bool parse_config_string(GKeyFile *config, const char *group,
return false;
}
- DBG("%s.%s = %s", group, key, tmp);
-
- if (val) {
- g_free(*val);
- *val = tmp;
- }
+ DBG("%s.%s = %s", group, key, *val);
return true;
}
@@ -1004,7 +1003,12 @@ static void parse_secure_conns(GKeyFile *config)
static void parse_general(GKeyFile *config)
{
- parse_config_string(config, "General", "Name", &btd_opts.name);
+ char *str = NULL;
+
+ if (parse_config_string(config, "General", "Name", &str)) {
+ g_free(btd_opts.name);
+ btd_opts.name = str;
+ }
parse_config_hex(config, "General", "Class", &btd_opts.class);
parse_config_u32(config, "General", "DiscoverableTimeout",
&btd_opts.discovto,
--
2.45.2

139
0001-shared-shell-Free-memory-allocated-by-wordexp.patch Normal file
View File

@ -0,0 +1,139 @@
From 9c7ec707e88170adf3e117fe92ed74e311b2e859 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Tue, 2 Jul 2024 15:27:12 +0200
Subject: [PATCH] shared/shell: Free memory allocated by wordexp()
Error: RESOURCE_LEAK (CWE-772): [#def38] [important]
bluez-5.76/src/shared/shell.c:519:2: alloc_arg: "parse_args" allocates memory that is stored into "w.we_wordv".
bluez-5.76/src/shared/shell.c:523:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
521| "Unable to parse mandatory command arguments: %s", man );
522| free(man);
523|-> return -EINVAL;
524| }
525|
Error: RESOURCE_LEAK (CWE-772): [#def40] [important]
bluez-5.76/src/shared/shell.c:1113:3: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv".
bluez-5.76/src/shared/shell.c:1114:4: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
1112|
1113| if (wordexp(rl_line_buffer, &w, WRDE_NOCMD))
1114|-> return NULL;
1115|
1116| matches = menu_completion(default_menu, text, w.we_wordc,
Error: RESOURCE_LEAK (CWE-772): [#def42] [important]
bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv".
bluez-5.76/src/shared/shell.c:1415:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
1413| switch (err) {
1414| case WRDE_BADCHAR:
1415|-> return -EBADMSG;
1416| case WRDE_BADVAL:
1417| case WRDE_SYNTAX:
Error: RESOURCE_LEAK (CWE-772): [#def43] [important]
bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv".
bluez-5.76/src/shared/shell.c:1418:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
1416| case WRDE_BADVAL:
1417| case WRDE_SYNTAX:
1418|-> return -EINVAL;
1419| case WRDE_NOSPACE:
1420| return -ENOMEM;
Error: RESOURCE_LEAK (CWE-772): [#def44] [important]
bluez-5.76/src/shared/shell.c:1412:2: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv".
bluez-5.76/src/shared/shell.c:1420:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
1418| return -EINVAL;
1419| case WRDE_NOSPACE:
1420|-> return -ENOMEM;
1421| case WRDE_CMDSUB:
1422| if (wordexp(input, &w, 0))
Error: RESOURCE_LEAK (CWE-772): [#def45] [important]
bluez-5.76/src/shared/shell.c:1422:3: alloc_arg: "wordexp" allocates memory that is stored into "w.we_wordv".
bluez-5.76/src/shared/shell.c:1423:4: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
1421| case WRDE_CMDSUB:
1422| if (wordexp(input, &w, 0))
1423|-> return -ENOEXEC;
1424| break;
1425| };
---
src/shared/shell.c | 22 ++++++++++++++++------
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/src/shared/shell.c b/src/shared/shell.c
index 88ecaa076adc..26c6a419af22 100644
--- a/src/shared/shell.c
+++ b/src/shared/shell.c
@@ -452,13 +452,23 @@ static void shell_print_menu_zsh_complete(void)
}
}
+static int _wordexp(const char *restrict s, wordexp_t *restrict p, int flags)
+{
+ int ret;
+
+ ret = wordexp(s, p, flags);
+ if (ret != 0)
+ wordfree(p);
+ return ret;
+}
+
static int parse_args(char *arg, wordexp_t *w, char *del, int flags)
{
char *str;
str = strdelimit(arg, del, '"');
- if (wordexp(str, w, flags)) {
+ if (_wordexp(str, w, flags) != 0) {
free(str);
return -EINVAL;
}
@@ -537,7 +547,7 @@ static int cmd_exec(const struct bt_shell_menu_entry *entry,
goto fail;
}
- flags |= WRDE_APPEND;
+ flags |= WRDE_APPEND | WRDE_REUSE;
opt = strdup(entry->arg + len + 1);
optional:
@@ -1043,7 +1053,7 @@ static char **args_completion(const struct bt_shell_menu_entry *entry, int argc,
args.we_offs = 0;
wordfree(&args);
- if (wordexp(str, &args, WRDE_NOCMD))
+ if (_wordexp(str, &args, WRDE_NOCMD))
goto done;
rl_completion_display_matches_hook = NULL;
@@ -1115,7 +1125,7 @@ static char **shell_completion(const char *text, int start, int end)
if (start > 0) {
wordexp_t w;
- if (wordexp(rl_line_buffer, &w, WRDE_NOCMD))
+ if (_wordexp(rl_line_buffer, &w, WRDE_NOCMD))
return NULL;
matches = menu_completion(default_menu, text, w.we_wordc,
@@ -1417,7 +1427,7 @@ int bt_shell_exec(const char *input)
if (data.monitor)
bt_log_printf(0xffff, data.name, LOG_INFO, "%s", input);
- err = wordexp(input, &w, WRDE_NOCMD);
+ err = _wordexp(input, &w, WRDE_NOCMD);
switch (err) {
case WRDE_BADCHAR:
return -EBADMSG;
@@ -1427,7 +1437,7 @@ int bt_shell_exec(const char *input)
case WRDE_NOSPACE:
return -ENOMEM;
case WRDE_CMDSUB:
- if (wordexp(input, &w, 0))
+ if (_wordexp(input, &w, 0))
return -ENOEXEC;
break;
};
--
2.45.2

3923
5.77-devel.patch Normal file
View File

File diff suppressed because it is too large Load Diff

13
bluez.spec
View File

@ -13,6 +13,17 @@ URL: http://www.bluez.org/
Source0: https://www.kernel.org/pub/linux/bluetooth/%{name}-%{version}.tar.xz
# Upstream patches
Patch0: 5.77-devel.patch
# https://patchwork.kernel.org/project/bluetooth/patch/20240702084900.773620-2-hadess@hadess.net/
Patch1: 0001-main-Simplify-parse_config_string.patch
# https://patchwork.kernel.org/project/bluetooth/patch/20240704102617.1132337-4-hadess@hadess.net/
Patch2: 0001-shared-shell-Free-memory-allocated-by-wordexp.patch
# https://patchwork.kernel.org/project/bluetooth/list/?series=876731
Patch3: static-analysis-issues-6.patch
# Coverity downstream patches
Patch4: coverity-workarounds.patch
BuildRequires: dbus-devel >= 1.6
BuildRequires: glib2-devel
BuildRequires: libell-devel >= 0.37
@ -330,6 +341,8 @@ install emulator/btvirt ${RPM_BUILD_ROOT}/%{_libexecdir}/bluetooth/
%changelog
* Mon Aug 05 2024 Bastien Nocera <bnocera@redhat.com> - 5.77-2
- Use git to apply patches
- Fix coverity issues
- Related: Jira:RHEL-34536
* Mon Aug 05 2024 Bastien Nocera <bnocera@redhat.com> - 5.77-1
- Update to 5.77

481
coverity-workarounds.patch Normal file
View File

@ -0,0 +1,481 @@
From ad622447efc5429a5dc3f84c722a81cc41658e7e Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Mon, 5 Aug 2024 12:17:29 +0200
Subject: [PATCH 1/8] monitor: Work-around overflow_sink Case #01164573
Coverity thinks "len" can be negative, even though we check its value,
and exit the function if it is.
---
monitor/control.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/monitor/control.c b/monitor/control.c
index 62857b4b84de..40e8a3a90c05 100644
--- a/monitor/control.c
+++ b/monitor/control.c
@@ -1102,6 +1102,7 @@ static void client_callback(int fd, uint32_t events, void *user_data)
UINT16_MAX - data->offset > len)
return;
+ /* coverity[overflow] : FALSE */
data->offset += len;
while (data->offset >= MGMT_HDR_SIZE) {
--
2.45.2
From c2a1630f0e484c4330c565c56e9a26f8f1ae2664 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Tue, 30 Jul 2024 15:45:18 +0200
Subject: [PATCH 2/8] mesh/net: Work-around memory overallocation warning
Coverity doesn't realise that the "payload" struct was allocated past
its structure size, so quiet that warning.
Error: OVERRUN (CWE-119): [#def1] [important]
bluez-5.77/mesh/net.c:3276:2: cond_at_most: Checking "msg_len > 384" implies that "msg_len" may be up to 384 on the false branch.
bluez-5.77/mesh/net.c:3290:2: cond_at_most: Checking "msg_len <= 15" implies that "msg_len" may be up to 15 on the true branch.
bluez-5.77/mesh/net.c:3316:2: overrun-buffer-arg: Overrunning array "payload->buf" of 4 bytes by passing it to a function which accesses it at byte offset 14 using argument "msg_len" (which evaluates to 15). [Note: The source code implementation of the function has been overridden by a builtin model.]
3314| /* Setup OTA Network send */
3315| payload = mesh_sar_new(msg_len);
3316|-> memcpy(payload->buf, msg, msg_len);
3317| payload->len = msg_len;
3318| payload->src = src;
---
mesh/net.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/mesh/net.c b/mesh/net.c
index ef6a3133859a..ca2cda8ec948 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -3306,6 +3306,7 @@ bool mesh_net_app_send(struct mesh_net *net, bool frnd_cred, uint16_t src,
/* Setup OTA Network send */
payload = mesh_sar_new(msg_len);
+ /* coverity[overrun-buffer-arg] : FALSE */
memcpy(payload->buf, msg, msg_len);
payload->len = msg_len;
payload->src = src;
--
2.45.2
From 6494fc8665f89b70b8e9d80b829eabc71a22278f Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Wed, 17 Jul 2024 12:51:56 +0200
Subject: [PATCH 3/8] shared/shell: Work-around SAT-45980 with wordexp()
Coverity sees a leak when one doesn't exist yet.
Error: RESOURCE_LEAK (CWE-772): [#def23] [important]
bluez-5.77/src/shared/shell.c:534:2: alloc_arg: "parse_args" allocates memory that is stored into "w.we_wordv".
bluez-5.77/src/shared/shell.c:558:3: leaked_storage: Variable "w" going out of scope leaks the storage "w.we_wordv" points to.
556| "Unable to parse optional command arguments: %s", opt);
557| free(opt);
558|-> return -EINVAL;
559| }
560|
---
src/shared/shell.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/shared/shell.c b/src/shared/shell.c
index 26c6a419af22..9d2b50b260f9 100644
--- a/src/shared/shell.c
+++ b/src/shared/shell.c
@@ -555,6 +555,7 @@ optional:
print_text(COLOR_HIGHLIGHT,
"Unable to parse optional command arguments: %s", opt);
free(opt);
+ /* coverity[leaked_storage : FALSE] */
return -EINVAL;
}
--
2.45.2
From 99c12a3e56129361ed50934054876126b1e55881 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Wed, 17 Jul 2024 11:28:17 +0200
Subject: [PATCH 4/8] sdp: Work-around #01163325 with single-linked list
Error: RESOURCE_LEAK (CWE-772): [#def2] [important]
bluez-5.77/lib/sdp.c:1896:4: alloc_fn: Storage is returned from allocation function "sdp_list_append".
bluez-5.77/lib/sdp.c:1896:4: var_assign: Assigning: "pds" = storage returned from "sdp_list_append(pds, curr->val.dataseq)".
bluez-5.77/lib/sdp.c:1896:4: identity_transfer: Passing "pds" as argument 1 to function "sdp_list_append", which returns that argument.
bluez-5.77/lib/sdp.c:1896:4: noescape: Resource "pds" is not freed or pointed-to in "sdp_list_append".
bluez-5.77/lib/sdp.c:1896:4: overwrite_var: Overwriting "pds" in "pds = sdp_list_append(pds, curr->val.dataseq)".
bluez-5.77/lib/sdp.c:1896:4: var_assign: Assigning: "pds" = storage returned from "sdp_list_append(pds, curr->val.dataseq)".
bluez-5.77/lib/sdp.c:1896:4: noescape: Resource "pds" is not freed or pointed-to in "sdp_list_append".
bluez-5.77/lib/sdp.c:1896:4: overwrite_var: Overwriting "pds" in "pds = sdp_list_append(pds, curr->val.dataseq)" leaks the storage that "pds" points to.
1894| goto failed;
1895| }
1896|-> pds = sdp_list_append(pds, curr->val.dataseq);
1897| }
1898|
Error: RESOURCE_LEAK (CWE-772): [#def3] [important]
bluez-5.77/lib/sdp.c:1899:3: alloc_fn: Storage is returned from allocation function "sdp_list_append".
bluez-5.77/lib/sdp.c:1899:3: var_assign: Assigning: "ap" = storage returned from "sdp_list_append(ap, pds)".
bluez-5.77/lib/sdp.c:1899:3: identity_transfer: Passing "ap" as argument 1 to function "sdp_list_append", which returns that argument.
bluez-5.77/lib/sdp.c:1899:3: noescape: Resource "ap" is not freed or pointed-to in "sdp_list_append".
bluez-5.77/lib/sdp.c:1899:3: overwrite_var: Overwriting "ap" in "ap = sdp_list_append(ap, pds)".
bluez-5.77/lib/sdp.c:1899:3: var_assign: Assigning: "ap" = storage returned from "sdp_list_append(ap, pds)".
bluez-5.77/lib/sdp.c:1899:3: noescape: Resource "ap" is not freed or pointed-to in "sdp_list_append".
bluez-5.77/lib/sdp.c:1899:3: overwrite_var: Overwriting "ap" in "ap = sdp_list_append(ap, pds)" leaks the storage that "ap" points to.
1897| }
1898|
1899|-> ap = sdp_list_append(ap, pds);
1900| }
1901|
Error: RESOURCE_LEAK (CWE-772): [#def17] [important]
bluez-5.77/src/sdp-client.c:197:3: alloc_fn: Storage is returned from allocation function "sdp_list_append".
bluez-5.77/src/sdp-client.c:197:3: var_assign: Assigning: "recs" = storage returned from "sdp_list_append(recs, rec)".
bluez-5.77/src/sdp-client.c:197:3: identity_transfer: Passing "recs" as argument 1 to function "sdp_list_append", which returns that argument.
bluez-5.77/src/sdp-client.c:197:3: noescape: Resource "recs" is not freed or pointed-to in "sdp_list_append".
bluez-5.77/src/sdp-client.c:197:3: overwrite_var: Overwriting "recs" in "recs = sdp_list_append(recs, rec)".
bluez-5.77/src/sdp-client.c:197:3: var_assign: Assigning: "recs" = storage returned from "sdp_list_append(recs, rec)".
bluez-5.77/src/sdp-client.c:197:3: noescape: Resource "recs" is not freed or pointed-to in "sdp_list_append".
bluez-5.77/src/sdp-client.c:197:3: overwrite_var: Overwriting "recs" in "recs = sdp_list_append(recs, rec)" leaks the storage that "recs" points to.
195| }
196|
197|-> recs = sdp_list_append(recs, rec);
198| } while (scanned < (ssize_t) size && bytesleft > 0);
199|
---
lib/sdp.c | 2 ++
src/sdp-client.c | 1 +
2 files changed, 3 insertions(+)
diff --git a/lib/sdp.c b/lib/sdp.c
index 8a15ad803db1..99efbc19c299 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1893,9 +1893,11 @@ static int sdp_get_proto_descs(uint16_t attr_id, const sdp_record_t *rec,
sdp_list_free(pds, NULL);
goto failed;
}
+ /* coverity[overwrite_var] : FALSE */
pds = sdp_list_append(pds, curr->val.dataseq);
}
+ /* coverity[overwrite_var] : FALSE */
ap = sdp_list_append(ap, pds);
}
diff --git a/src/sdp-client.c b/src/sdp-client.c
index 71d3d9e95044..2f043cb7f010 100644
--- a/src/sdp-client.c
+++ b/src/sdp-client.c
@@ -194,6 +194,7 @@ static void search_completed_cb(uint8_t type, uint16_t status,
continue;
}
+ /* coverity[overwrite_var] : FALSE */
recs = sdp_list_append(recs, rec);
} while (scanned < (ssize_t) size && bytesleft > 0);
--
2.45.2
From 6fcbf34a02133628a1a8afeabb093270ca89dbb8 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Thu, 18 Jul 2024 15:05:07 +0200
Subject: [PATCH 5/8] mesh: Quiet imprecise "overrun-buffer-val" #01163326
Error: OVERRUN (CWE-119): [#def1] [important]
bluez-5.77/mesh/friend.c:326:2: overrun-buffer-val: Overrunning array "msg" of 5 bytes by passing it to a function which accesses it at byte offset 12.
324| l_put_be16(neg->lp_addr, msg + 1);
325| l_put_be16(neg->lp_cnt, msg + 3);
326|-> mesh_net_transport_send(neg->net, 0, 0,
327| mesh_net_get_iv_index(neg->net), DEFAULT_TTL,
328| 0, 0, neg->old_friend,
Error: OVERRUN (CWE-119): [#def2] [important]
bluez-5.77/mesh/net.c:276:2: overrun-buffer-val: Overrunning array "msg" of 4 bytes by passing it to a function which accesses it at byte offset 12.
274| n += 2;
275|
276|-> mesh_net_transport_send(net, 0, 0, mesh_net_get_iv_index(net),
277| pub->ttl, 0, 0, pub->dst, msg, n);
278| }
Error: OVERRUN (CWE-119): [#def3] [important]
bluez-5.77/mesh/net.c:1463:3: overrun-buffer-val: Overrunning array "msg" of 7 bytes by passing it to a function which accesses it at byte offset 12.
1461| mesh_net_next_seq_num(net), 0, dst, msg);
1462| } else {
1463|-> mesh_net_transport_send(net, 0, 0,
1464| mesh_net_get_iv_index(net), DEFAULT_TTL,
1465| 0, 0, dst, msg, sizeof(msg));
Error: OVERRUN (CWE-119): [#def4] [important]
bluez-5.77/mesh/net.c:1498:2: overrun-buffer-val: Overrunning array "msg" of 7 bytes by passing it to a function which accesses it at byte offset 12.
1496| }
1497|
1498|-> mesh_net_transport_send(net, 0, sar->net_idx,
1499| mesh_net_get_iv_index(net), DEFAULT_TTL,
1500| 0, src, dst, msg,
Error: OVERRUN (CWE-119): [#def6] [important]
bluez-5.77/mesh/net.c:2053:3: overrun-buffer-val: Overrunning array "sar_in->buf" of 4 bytes by passing it to a function which accesses it at byte offset 11.
2051| send_net_ack(net, sar_in, expected);
2052|
2053|-> msg_rxed(net, frnd, iv_index, ttl, seq, net_idx,
2054| sar_in->remote, dst, key_aid, true, szmic,
2055| sar_in->seqZero, sar_in->buf, sar_in->len);
---
mesh/friend.c | 1 +
mesh/net.c | 4 ++++
2 files changed, 5 insertions(+)
diff --git a/mesh/friend.c b/mesh/friend.c
index 5b73da68916f..bb8f62e9f57f 100644
--- a/mesh/friend.c
+++ b/mesh/friend.c
@@ -323,6 +323,7 @@ static void clear_retry(struct l_timeout *timeout, void *user_data)
l_put_be16(neg->lp_addr, msg + 1);
l_put_be16(neg->lp_cnt, msg + 3);
+ /* coverity[overrun-buffer-val] : FALSE */
mesh_net_transport_send(neg->net, 0, 0,
mesh_net_get_iv_index(neg->net), DEFAULT_TTL,
0, 0, neg->old_friend,
diff --git a/mesh/net.c b/mesh/net.c
index ca2cda8ec948..9d6c2ae5142f 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -273,6 +273,7 @@ static void send_hb_publication(void *data)
l_put_be16(net->features, msg + n);
n += 2;
+ /* coverity[overrun-buffer-val] : FALSE */
mesh_net_transport_send(net, 0, 0, mesh_net_get_iv_index(net),
pub->ttl, 0, 0, pub->dst, msg, n);
}
@@ -1460,6 +1461,7 @@ static void send_frnd_ack(struct mesh_net *net, uint16_t src, uint16_t dst,
friend_ack_rxed(net, mesh_net_get_iv_index(net),
mesh_net_next_seq_num(net), 0, dst, msg);
} else {
+ /* coverity[overrun-buffer-val] : FALSE */
mesh_net_transport_send(net, 0, 0,
mesh_net_get_iv_index(net), DEFAULT_TTL,
0, 0, dst, msg, sizeof(msg));
@@ -1495,6 +1497,7 @@ static void send_net_ack(struct mesh_net *net, struct mesh_sar *sar,
return;
}
+ /* coverity[overrun-buffer-val] : FALSE */
mesh_net_transport_send(net, 0, sar->net_idx,
mesh_net_get_iv_index(net), DEFAULT_TTL,
0, src, dst, msg,
@@ -2050,6 +2053,7 @@ static bool seg_rxed(struct mesh_net *net, bool frnd, uint32_t iv_index,
/* Got it all */
send_net_ack(net, sar_in, expected);
+ /* coverity[overrun-buffer-val] : FALSE */
msg_rxed(net, frnd, iv_index, ttl, seq, net_idx,
sar_in->remote, dst, key_aid, true, szmic,
sar_in->seqZero, sar_in->buf, sar_in->len);
--
2.45.2
From 91066706378840f28146e51702e3ed8c1780dcd9 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Thu, 18 Jul 2024 15:37:58 +0200
Subject: [PATCH 6/8] mesh: Quiet imprecise "overrun-buffer-val" #01163327
Those errors are incorrect, as just before the flagged function calls,
the packet is modified to flag for a "segmented" packet, which is
handled differently, so nothing is accessed past the array size.
Error: OVERRUN (CWE-119): [#def5] [important]
bluez-5.77/mesh/net.c:1769:3: cond_at_least: Checking "size > 15" implies that "size" is at least 16 on the true branch.
bluez-5.77/mesh/net.c:1776:3: overrun-call: Overrunning callee's array of size 15 by passing argument "size" (which evaluates to 16) in call to "friend_packet_queue".
1774| }
1775|
1776|-> if (friend_packet_queue(net, iv_index, false, frnd_ttl,
1777| seq, src, dst,
1778| hdr, data, size))
Error: OVERRUN (CWE-119): [#def2] [important]
bluez-5.77/mesh/net.c:2016:3: cond_at_least: Checking "segN" implies that "segN" is at least 1 on the true branch.
bluez-5.77/mesh/net.c:2016:3: assignment: Assigning: "len" = "segN ? (segN + 1) * 12 : 15". The value of "len" is now at least 24.
bluez-5.77/mesh/net.c:2028:3: assignment: Assigning: "sar_in->len" = "len". The value of "sar_in->len" is now at least 24.
bluez-5.77/mesh/net.c:2058:3: overrun-call: Overrunning callee's array of size 15 by passing argument "sar_in->len" (which evaluates to 24) in call to "msg_rxed".
2056|
2057| /* coverity[overrun-buffer-val] : FALSE */
2058|-> msg_rxed(net, frnd, iv_index, ttl, seq, net_idx,
2059| sar_in->remote, dst, key_aid, true, szmic,
2060| sar_in->seqZero, sar_in->buf, sar_in->len);
Error: OVERRUN (CWE-119): [#def4] [important]
bluez-5.77/mesh/net.c:3266:2: cond_at_most: Checking "msg_len > 384" implies that "msg_len" may be up to 384 on the false branch.
bluez-5.77/mesh/net.c:3280:2: cond_between: Checking "msg_len <= 15" implies that "msg_len" is between 16 and 384 (inclusive) on the false branch.
bluez-5.77/mesh/net.c:3284:2: overrun-call: Overrunning callee's array of size 15 by passing argument "msg_len" (which evaluates to 384) in call to "msg_rxed".
3282|
3283| /* First enqueue to any Friends and internal models */
3284|-> result = msg_rxed(net, false, iv_index, ttl, seq, net_idx, src, dst,
3285| key_aid, segmented, szmic, seq & SEQ_ZERO_MASK,
3286| msg, msg_len);
---
mesh/net.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/mesh/net.c b/mesh/net.c
index 9d6c2ae5142f..30dcdb2fe517 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -1776,6 +1776,7 @@ static bool msg_rxed(struct mesh_net *net, bool frnd, uint32_t iv_index,
hdr |= SEG_MAX(true, size) << SEGN_HDR_SHIFT;
}
+ /* coverity[overrun-call] : FALSE */
if (friend_packet_queue(net, iv_index, false, frnd_ttl,
seq, src, dst,
hdr, data, size))
@@ -2054,6 +2055,7 @@ static bool seg_rxed(struct mesh_net *net, bool frnd, uint32_t iv_index,
send_net_ack(net, sar_in, expected);
/* coverity[overrun-buffer-val] : FALSE */
+ /* coverity[overrun-call] : FALSE */
msg_rxed(net, frnd, iv_index, ttl, seq, net_idx,
sar_in->remote, dst, key_aid, true, szmic,
sar_in->seqZero, sar_in->buf, sar_in->len);
@@ -3289,6 +3291,7 @@ bool mesh_net_app_send(struct mesh_net *net, bool frnd_cred, uint16_t src,
segmented |= !!(seg_max);
/* First enqueue to any Friends and internal models */
+ /* coverity[overrun-call] : FALSE */
result = msg_rxed(net, false, iv_index, ttl, seq, net_idx, src, dst,
key_aid, segmented, szmic, seq & SEQ_ZERO_MASK,
msg, msg_len);
--
2.45.2
From 1a1239f998ca15dd233e2adaa2ce12f4ae97e5d1 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Fri, 19 Jul 2024 15:06:24 +0200
Subject: [PATCH 7/8] shared/gatt-db: Work-around overrun-buffer-arg case
#01163328
Despite the checks added, Coverity still thinks that uuid_to_le() can
return more than 16 (for UUID128 / 8), so quiet those.
Error: OVERRUN (CWE-119): [#def6] [important]
bluez-5.77/src/shared/gatt-db.c:612:2: assignment: Assigning: "len" = "uuid_to_le(uuid, value)". The value of "len" is now between 0 and 31 (inclusive).
bluez-5.77/src/shared/gatt-db.c:614:2: overrun-buffer-arg: Overrunning array "value" of 16 bytes by passing it to a function which accesses it at byte offset 30 using argument "len" (which evaluates to 31).
612| len = uuid_to_le(uuid, value);
613|
614|-> service->attributes[0] = new_attribute(service, handle, type, value,
615| len);
616| if (!service->attributes[0]) {
Error: OVERRUN (CWE-119): [#def7] [important]
bluez-5.77/src/shared/gatt-db.c:947:2: assignment: Assigning: "len" = "0".
bluez-5.77/src/shared/gatt-db.c:971:2: assignment: Assigning: "len" += "1UL". The value of "len" is now 1.
bluez-5.77/src/shared/gatt-db.c:975:2: assignment: Assigning: "len" += "2UL". The value of "len" is now 3.
bluez-5.77/src/shared/gatt-db.c:976:2: assignment: Assigning: "len" += "uuid_to_le(uuid, &value[3])". The value of "len" is now between 3 and 34 (inclusive).
bluez-5.77/src/shared/gatt-db.c:978:2: overrun-buffer-arg: Overrunning array "value" of 19 bytes by passing it to a function which accesses it at byte offset 33 using argument "len" (which evaluates to 34).
976| len += uuid_to_le(uuid, &value[3]);
977|
978|-> service->attributes[i] = new_attribute(service, handle,
979| &characteristic_uuid,
980| value, len);
Error: OVERRUN (CWE-119): [#def8] [important]
bluez-5.77/src/shared/gatt-db.c:947:2: assignment: Assigning: "len" = "0".
bluez-5.77/src/shared/gatt-db.c:971:2: assignment: Assigning: "len" += "1UL". The value of "len" is now 1.
bluez-5.77/src/shared/gatt-db.c:975:2: assignment: Assigning: "len" += "2UL". The value of "len" is now 3.
bluez-5.77/src/shared/gatt-db.c:976:2: assignment: Assigning: "len" += "uuid_to_le(uuid, &value[3])". The value of "len" is now between 3 and 34 (inclusive).
bluez-5.77/src/shared/gatt-db.c:1005:2: overrun-buffer-arg: Overrunning array "value" of 19 bytes by passing it to a function which accesses it at byte offset 33 using argument "len" (which evaluates to 34).
1003| /* Update handle of characteristic value_handle if it has changed */
1004| put_le16(value_handle, &value[1]);
1005|-> if (memcmp((*chrc)->value, value, len))
1006| memcpy((*chrc)->value, value, len);
1007|
Error: OVERRUN (CWE-119): [#def9] [important]
bluez-5.77/src/shared/gatt-db.c:947:2: assignment: Assigning: "len" = "0".
bluez-5.77/src/shared/gatt-db.c:971:2: assignment: Assigning: "len" += "1UL". The value of "len" is now 1.
bluez-5.77/src/shared/gatt-db.c:975:2: assignment: Assigning: "len" += "2UL". The value of "len" is now 3.
bluez-5.77/src/shared/gatt-db.c:976:2: assignment: Assigning: "len" += "uuid_to_le(uuid, &value[3])". The value of "len" is now between 3 and 34 (inclusive).
bluez-5.77/src/shared/gatt-db.c:1006:3: overrun-buffer-arg: Overrunning array "value" of 19 bytes by passing it to a function which accesses it at byte offset 33 using argument "len" (which evaluates to 34). [Note: The source code implementation of the function has been overridden by a builtin model.]
1004| put_le16(value_handle, &value[1]);
1005| if (memcmp((*chrc)->value, value, len))
1006|-> memcpy((*chrc)->value, value, len);
1007|
1008| set_attribute_data(service->attributes[i], read_func, write_func,
---
src/shared/gatt-db.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index cd0eba6bf1d0..9045a53c6dfe 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -616,6 +616,7 @@ static struct gatt_db_service *gatt_db_service_create(const bt_uuid_t *uuid,
len = uuid_to_le(uuid, value);
+ /* coverity[overrun-buffer-arg] : FALSE */
service->attributes[0] = new_attribute(service, handle, type, value,
len);
if (!service->attributes[0]) {
@@ -980,6 +981,7 @@ service_insert_characteristic(struct gatt_db_service *service,
len += sizeof(uint16_t);
len += uuid_to_le(uuid, &value[3]);
+ /* coverity[overrun-buffer-arg] : FALSE */
service->attributes[i] = new_attribute(service, handle,
&characteristic_uuid,
value, len);
@@ -1007,8 +1009,11 @@ service_insert_characteristic(struct gatt_db_service *service,
/* Update handle of characteristic value_handle if it has changed */
put_le16(value_handle, &value[1]);
- if (memcmp((*chrc)->value, value, len))
+ /* coverity[overrun-buffer-arg] : FALSE */
+ if (memcmp((*chrc)->value, value, len)) {
+ /* coverity[overrun-buffer-arg] : FALSE */
memcpy((*chrc)->value, value, len);
+ }
set_attribute_data(service->attributes[i], read_func, write_func,
permissions, user_data);
--
2.45.2
From cddd78cb6d2a780b352e27ea5e7e44378f8a8ef4 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Tue, 30 Jul 2024 15:27:49 +0200
Subject: [PATCH 8/8] shared/btsnoop: Work-around underflow case #01163329
It should be impossible to have toread underflow, as we check that it
has a value of at least 1 when decremented, and that we check for it
have a non-zero value before using it.
---
src/shared/btsnoop.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/src/shared/btsnoop.c b/src/shared/btsnoop.c
index bb0bccf0dd01..12f960ec353d 100644
--- a/src/shared/btsnoop.c
+++ b/src/shared/btsnoop.c
@@ -553,6 +553,7 @@ bool btsnoop_read_hci(struct btsnoop *btsnoop, struct timeval *tv,
btsnoop->aborted = true;
return false;
}
+ /* coverity[underflow] : FALSE */
toread--;
*index = 0;
--
2.45.2

353
static-analysis-issues-6.patch Normal file
View File

@ -0,0 +1,353 @@
From bdf5fd2a0156e9070e1e55777b4a71033160fbf1 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Wed, 17 Jul 2024 12:37:16 +0200
Subject: [PATCH 1/8] sdp: Ensure size doesn't overflow
Error: INTEGER_OVERFLOW (CWE-190): [#def1] [important]
bluez-5.77/lib/sdp.c:1685:2: tainted_data_argument: The check "sent < size" contains the tainted expression "sent" which causes "size" to be considered tainted.
bluez-5.77/lib/sdp.c:1686:3: overflow: The expression "size - sent" is deemed overflowed because at least one of its arguments has overflowed.
bluez-5.77/lib/sdp.c:1686:3: overflow_sink: "size - sent", which might have underflowed, is passed to "send(session->sock, buf + sent, size - sent, 0)".
1684|
1685| while (sent < size) {
1686|-> int n = send(session->sock, buf + sent, size - sent, 0);
1687| if (n < 0)
1688| return -1;
---
lib/sdp.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/lib/sdp.c b/lib/sdp.c
index 411a95b8a7d3..8a15ad803db1 100644
--- a/lib/sdp.c
+++ b/lib/sdp.c
@@ -1678,13 +1678,13 @@ sdp_data_t *sdp_data_get(const sdp_record_t *rec, uint16_t attrId)
return NULL;
}
-static int sdp_send_req(sdp_session_t *session, uint8_t *buf, uint32_t size)
+static int sdp_send_req(sdp_session_t *session, uint8_t *buf, size_t size)
{
- uint32_t sent = 0;
+ size_t sent = 0;
while (sent < size) {
int n = send(session->sock, buf + sent, size - sent, 0);
- if (n < 0)
+ if (n < 0 || sent > SIZE_MAX - n)
return -1;
sent += n;
}
--
2.45.2
From 062c998fb5c407bc09d6124324b1bd393997bfee Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Thu, 18 Jul 2024 15:43:35 +0200
Subject: [PATCH 2/8] tools/isotest: Ensure ret doesn't overflow
Error: INTEGER_OVERFLOW (CWE-190): [#def20] [important]
bluez-5.77/tools/isotest.c:778:2: tainted_data_argument: The check "ret < count" contains the tainted expression "ret" which causes "count" to be considered tainted.
bluez-5.77/tools/isotest.c:779:3: overflow: The expression "count - ret" is deemed overflowed because at least one of its arguments has overflowed.
bluez-5.77/tools/isotest.c:779:3: overflow_sink: "count - ret", which might have underflowed, is passed to "read(fd, buf + ret, count - ret)". [Note: The source code implementation of the function has been overridden by a builtin model.]
777|
778| while (ret < count) {
779|-> len = read(fd, buf + ret, count - ret);
780| if (len < 0)
781| return -errno;
---
tools/isotest.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/tools/isotest.c b/tools/isotest.c
index 2cac0e49cc39..0805faa66e47 100644
--- a/tools/isotest.c
+++ b/tools/isotest.c
@@ -779,6 +779,8 @@ static int read_stream(int fd, ssize_t count)
len = read(fd, buf + ret, count - ret);
if (len < 0)
return -errno;
+ if (len > SSIZE_MAX - ret)
+ return -EOVERFLOW;
ret += len;
usleep(1000);
--
2.45.2
From 122a888962765010162306f19fccf77333e1bc1b Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Thu, 18 Jul 2024 15:45:47 +0200
Subject: [PATCH 3/8] health: mcap: Ensure sent doesn't overflow
Error: INTEGER_OVERFLOW (CWE-190): [#def13] [important]
bluez-5.77/profiles/health/mcap.c:390:2: tainted_data_argument: The check "sent < size" contains the tainted expression "sent" which causes "size" to be considered tainted.
bluez-5.77/profiles/health/mcap.c:391:3: overflow: The expression "size - sent" is deemed overflowed because at least one of its arguments has overflowed.
bluez-5.77/profiles/health/mcap.c:391:3: overflow_sink: "size - sent", which might have underflowed, is passed to "write(sock, buf_b + sent, size - sent)".
389|
390| while (sent < size) {
391|-> int n = write(sock, buf_b + sent, size - sent);
392| if (n < 0)
393| return -1;
---
profiles/health/mcap.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/profiles/health/mcap.c b/profiles/health/mcap.c
index 2e4214a6984f..b3bf403e74d2 100644
--- a/profiles/health/mcap.c
+++ b/profiles/health/mcap.c
@@ -389,7 +389,7 @@ int mcap_send_data(int sock, const void *buf, uint32_t size)
while (sent < size) {
int n = write(sock, buf_b + sent, size - sent);
- if (n < 0)
+ if (n < 0 || n > SSIZE_MAX - sent)
return -1;
sent += n;
}
--
2.45.2
From fce37c2100a043fce99fbe2e8c8171406b841fae Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Fri, 19 Jul 2024 11:26:45 +0200
Subject: [PATCH 4/8] shared/tester: Add early failure check
Add a similar assertion to the other tests to avoid passing negative len
to tester_monitor() which might result in crashes.
Error: OVERRUN (CWE-119): [#def13] [important]
bluez-5.77/src/shared/tester.c:946:2: return_constant: Function call "io_send(io, iov, 1)" may return -107.
bluez-5.77/src/shared/tester.c:946:2: assignment: Assigning: "len" = "io_send(io, iov, 1)". The value of "len" is now -107.
bluez-5.77/src/shared/tester.c:948:2: overrun-buffer-arg: Calling "tester_monitor" with "iov->iov_base" and "len" is suspicious because of the very large index, 18446744073709551509. The index may be due to a negative parameter being interpreted as unsigned.
946| len = io_send(io, iov, 1);
947|
948|-> tester_monitor('<', 0x0004, 0x0000, iov->iov_base, len);
949|
950| g_assert_cmpint(len, ==, iov->iov_len);
---
src/shared/tester.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/shared/tester.c b/src/shared/tester.c
index 56c8cba6f578..3053025d7945 100644
--- a/src/shared/tester.c
+++ b/src/shared/tester.c
@@ -945,6 +945,8 @@ static bool test_io_send(struct io *io, void *user_data)
len = io_send(io, iov, 1);
+ g_assert(len > 0);
+
tester_monitor('<', 0x0004, 0x0000, iov->iov_base, len);
g_assert_cmpint(len, ==, iov->iov_len);
--
2.45.2
From 5078e205d5892048cb1243ce2977dcf8eb0c02fc Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Mon, 29 Jul 2024 13:53:41 +0200
Subject: [PATCH 5/8] mesh: Fix possible integer overflow
Error: INTEGER_OVERFLOW (CWE-190): [#def1] [important]
bluez-5.77/mesh/net.c:3164:4: cast_overflow: Truncation due to cast operation on "msg->len - seg_off" from 32 to 8 bits.
bluez-5.77/mesh/net.c:3164:4: overflow_assign: "seg_len" is assigned from "msg->len - seg_off".
bluez-5.77/mesh/net.c:3178:2: overflow_sink: "seg_len", which might have overflowed, is passed to "mesh_crypto_packet_build(false, msg->ttl, seq_num, msg->src, msg->remote, 0, msg->segmented, msg->key_aid, msg->szmic, false, msg->seqZero, segO, segN, msg->buf + seg_off, seg_len, packet + 1, &packet_len)".
3176|
3177| /* TODO: Are we RXing on an LPN's behalf? Then set RLY bit */
3178|-> if (!mesh_crypto_packet_build(false, msg->ttl, seq_num, msg->src,
3179| msg->remote, 0, msg->segmented,
3180| msg->key_aid, msg->szmic, false,
X
---
mesh/net.c | 13 +++++++++++--
1 file changed, 11 insertions(+), 2 deletions(-)
diff --git a/mesh/net.c b/mesh/net.c
index 05ca48326fc5..ef6a3133859a 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -3149,13 +3149,22 @@ static bool send_seg(struct mesh_net *net, uint8_t cnt, uint16_t interval,
uint32_t seq_num;
if (msg->segmented) {
+ if (msg->len < seg_off) {
+ l_error("Failed to build packet");
+ return false;
+ }
/* Send each segment on unique seq_num */
seq_num = mesh_net_next_seq_num(net);
- if (msg->len - seg_off > SEG_OFF(1))
+ if (msg->len - seg_off > SEG_OFF(1)) {
seg_len = SEG_OFF(1);
- else
+ } else {
+ if (msg->len - seg_off > UINT8_MAX) {
+ l_error("Failed to build packet");
+ return false;
+ }
seg_len = msg->len - seg_off;
+ }
} else {
/* Send on same seq_num used for Access Layer */
seq_num = msg->seqAuth;
--
2.45.2
From c37f2cdd4b8fa66fc97d423c4c980865b4793ef2 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Fri, 19 Jul 2024 14:27:54 +0200
Subject: [PATCH 6/8] shared/gatt-db: Fix possible buffer overrun
uuid_to_le() returns one of the possible values from bt_uuid_len().
bt_uuid_len() returns "type / 8".
type is a value between 0 and 128, but could be something else
depending on the validity of the UUID that's parsed. So an invalid
value of type between 128 and 256 would trigger an overrun.
Add a check to make sure that an invalid type isn't used to calculate
the length.
Error: OVERRUN (CWE-119): [#def6] [important]
bluez-5.77/src/shared/gatt-db.c:612:2: assignment: Assigning: "len" = "uuid_to_le(uuid, value)". The value of "len" is now between 0 and 31 (inclusive).
bluez-5.77/src/shared/gatt-db.c:614:2: overrun-buffer-arg: Overrunning array "value" of 16 bytes by passing it to a function which accesses it at byte offset 30 using argument "len" (which evaluates to 31).
612| len = uuid_to_le(uuid, value);
613|
614|-> service->attributes[0] = new_attribute(service, handle, type, value,
615| len);
616| if (!service->attributes[0]) {
---
src/shared/gatt-db.c | 11 ++++++++---
1 file changed, 8 insertions(+), 3 deletions(-)
diff --git a/src/shared/gatt-db.c b/src/shared/gatt-db.c
index b35763410d17..cd0eba6bf1d0 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -560,9 +560,14 @@ static int uuid_to_le(const bt_uuid_t *uuid, uint8_t *dst)
return bt_uuid_len(uuid);
}
- bt_uuid_to_uuid128(uuid, &uuid128);
- bswap_128(&uuid128.value.u128, dst);
- return bt_uuid_len(&uuid128);
+ if (uuid->type == BT_UUID32 ||
+ uuid->type == BT_UUID128) {
+ bt_uuid_to_uuid128(uuid, &uuid128);
+ bswap_128(&uuid128.value.u128, dst);
+ return bt_uuid_len(&uuid128);
+ }
+
+ return 0;
}
static bool le_to_uuid(const uint8_t *src, size_t len, bt_uuid_t *uuid)
--
2.45.2
From b7cb9a4bc9b94ded15be812d1d444d0ace4a886d Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Fri, 19 Jul 2024 11:29:15 +0200
Subject: [PATCH 7/8] shared/btsnoop: Avoid underflowing toread variable
Error: INTEGER_OVERFLOW (CWE-190): [#def8] [important]
bluez-5.77/src/shared/btsnoop.c:556:3: underflow: The decrement operator on the unsigned variable "toread" might result in an underflow.
bluez-5.77/src/shared/btsnoop.c:572:2: overflow_sink: "toread", which might have underflowed, is passed to "read(btsnoop->fd, data, toread)". [Note: The source code implementation of the function has been overridden by a builtin model.]
570| }
571|
572|-> len = read(btsnoop->fd, data, toread);
573| if (len < 0) {
574| btsnoop->aborted = true;
---
src/shared/btsnoop.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/src/shared/btsnoop.c b/src/shared/btsnoop.c
index bc5f7fcbe84c..bb0bccf0dd01 100644
--- a/src/shared/btsnoop.c
+++ b/src/shared/btsnoop.c
@@ -530,7 +530,7 @@ bool btsnoop_read_hci(struct btsnoop *btsnoop, struct timeval *tv,
}
toread = be32toh(pkt.len);
- if (toread > BTSNOOP_MAX_PACKET_SIZE) {
+ if (toread > BTSNOOP_MAX_PACKET_SIZE || toread < 1) {
btsnoop->aborted = true;
return false;
}
@@ -569,6 +569,11 @@ bool btsnoop_read_hci(struct btsnoop *btsnoop, struct timeval *tv,
return false;
}
+ if (toread == 0) {
+ btsnoop->aborted = true;
+ return false;
+ }
+
len = read(btsnoop->fd, data, toread);
if (len < 0) {
btsnoop->aborted = true;
--
2.45.2
From 354babc88eb98970a9f59056b41854b0f0f87859 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Fri, 19 Jul 2024 15:14:26 +0200
Subject: [PATCH 8/8] monitor: Check for possible integer underflow
Error: INTEGER_OVERFLOW (CWE-190): [#def4] [important]
bluez-5.77/monitor/control.c:1094:2: tainted_data_return: Called function "recv(data->fd, data->buf + data->offset, 1490UL - data->offset, MSG_DONTWAIT)", and a possible return value may be less than zero.
bluez-5.77/monitor/control.c:1094:2: assign: Assigning: "len" = "recv(data->fd, data->buf + data->offset, 1490UL - data->offset, MSG_DONTWAIT)".
bluez-5.77/monitor/control.c:1099:2: overflow: The expression "data->offset" is considered to have possibly overflowed.
bluez-5.77/monitor/control.c:1115:3: overflow: The expression "data->offset -= pktlen + 6" is deemed overflowed because at least one of its arguments has overflowed.
bluez-5.77/monitor/control.c:1118:4: overflow_sink: "data->offset", which might have underflowed, is passed to "memmove(data->buf, data->buf + 6 + pktlen, data->offset)". [Note: The source code implementation of the function has been overridden by a builtin model.]
1116|
1117| if (data->offset > 0)
1118|-> memmove(data->buf, data->buf + MGMT_HDR_SIZE + pktlen,
1119| data->offset);
1120| }
---
monitor/control.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/monitor/control.c b/monitor/control.c
index 009cf15209f0..62857b4b84de 100644
--- a/monitor/control.c
+++ b/monitor/control.c
@@ -18,6 +18,7 @@
#include <stdbool.h>
#include <stddef.h>
#include <errno.h>
+#include <limits.h>
#include <unistd.h>
#include <stdlib.h>
#include <string.h>
@@ -1091,9 +1092,14 @@ static void client_callback(int fd, uint32_t events, void *user_data)
return;
}
+ if (sizeof(data->buf) <= data->offset)
+ return;
+
len = recv(data->fd, data->buf + data->offset,
sizeof(data->buf) - data->offset, MSG_DONTWAIT);
- if (len < 0)
+ if (len < 0 ||
+ len > UINT16_MAX ||
+ UINT16_MAX - data->offset > len)
return;
data->offset += len;
--
2.45.2