rpms/bluez
6
0
Fork 0
Code Issues Pull Requests Releases Activity
e5229e5c43
bluez/static-analysis-issues-6.patch
Bastien Nocera 1886ee4181 Update to 5.83 
Resolves: RHEL-94817
2025-06-16 15:39:43 +02:00

354 lines
13 KiB
Diff
Raw Blame History

From 36acca0e4f94bb926b16c2fe4536d416cdbf43e6 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.49.0
From 70c6433630f168d46af905f3721663f8db475558 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.49.0
From ce244bd2d24e95017df2e1dea6c3e90a662d05da 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.49.0
From cac8152c9e741c7d7c939bd755c55295338b0a7e 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 371ccacedc66..1086a80363db 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.49.0
From 916115b8a4aeab9d10a2984a6593d93dcf5a30c2 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 cc862dade5ae..2483e23c4229 100644
--- a/mesh/net.c
+++ b/mesh/net.c
@@ -3150,13 +3150,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.49.0
From 51a597594dadb8150348d3a9ecb403db0cc75316 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 8951079beef1..c990dcd057eb 100644
--- a/src/shared/gatt-db.c
+++ b/src/shared/gatt-db.c
@@ -566,9 +566,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.49.0
From 4b38628a0602417a79e50a0d9219e3b8917082de 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.49.0
From bf3eaaef06637a1597e86e03f10c0eb8b3b0139e 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.49.0
Reference in New Issue View Git Blame Copy Permalink