From: Tedd Ho-Jeong An <[email protected]>
The Coverity scan reported (CWE-676):
rand() should not be used for security-related applications, because
linear congruential algorithms are too easy to break.
This series of patch replaces the standard random number generation
function, rand(), to getrandom() syscall, which provides more secure
random number than the standard rand() function.
Tedd Ho-Jeong An (5):
emulator: Replace random number generation function
peripheral: Replace random number generation function
tools/btgatt-server: Replace random number generation function
plugins: Replace random number generation function
profiles/health: Replace random number generation function
emulator/le.c | 11 +++++++++--
emulator/phy.c | 10 ++++++++--
peripheral/main.c | 11 ++++++-----
plugins/autopair.c | 8 +++++++-
profiles/health/hdp.c | 11 +++++++----
profiles/health/mcap.c | 17 +++++++++++++++--
tools/btgatt-server.c | 7 ++++++-
7 files changed, 58 insertions(+), 17 deletions(-)
--
2.25.1
From: Tedd Ho-Jeong An <[email protected]>
This patch replaces the rand() function to the getrandom() syscall.
It was reported by the Coverity scan
rand() should not be used for security-related applications, because
linear congruential algorithms are too easy to break
---
emulator/le.c | 11 +++++++++--
emulator/phy.c | 10 ++++++++--
2 files changed, 17 insertions(+), 4 deletions(-)
diff --git a/emulator/le.c b/emulator/le.c
index 07a44c5f1..f8f313f2c 100644
--- a/emulator/le.c
+++ b/emulator/le.c
@@ -20,6 +20,7 @@
#include <sys/socket.h>
#include <sys/un.h>
#include <sys/uio.h>
+#include <sys/random.h>
#include <time.h>
#include "lib/bluetooth.h"
@@ -503,11 +504,17 @@ static void send_adv_pkt(struct bt_le *hci, uint8_t channel)
static unsigned int get_adv_delay(void)
{
+ unsigned int val;
+
/* The advertising delay is a pseudo-random value with a range
* of 0 ms to 10 ms generated for each advertising event.
*/
- srand(time(NULL));
- return (rand() % 11);
+ if (getrandom(&val, sizeof(val), 0) < 0) {
+ /* If it fails to get the random number, use a static value */
+ val = 5;
+ }
+
+ return (val % 11);
}
static void adv_timeout_callback(int id, void *user_data)
diff --git a/emulator/phy.c b/emulator/phy.c
index 2ae6ad3a2..44cace438 100644
--- a/emulator/phy.c
+++ b/emulator/phy.c
@@ -19,6 +19,7 @@
#include <stdlib.h>
#include <string.h>
#include <sys/socket.h>
+#include <sys/random.h>
#include <netinet/in.h>
#include <netinet/ip.h>
#include <time.h>
@@ -173,8 +174,13 @@ struct bt_phy *bt_phy_new(void)
mainloop_add_fd(phy->rx_fd, EPOLLIN, phy_rx_callback, phy, NULL);
if (!get_random_bytes(&phy->id, sizeof(phy->id))) {
- srandom(time(NULL));
- phy->id = random();
+ if (getrandom(&phy->id, sizeof(phy->id), 0) < 0) {
+ mainloop_remove_fd(phy->rx_fd);
+ close(phy->tx_fd);
+ close(phy->rx_fd);
+ free(phy);
+ return NULL;
+ }
}
bt_phy_send(phy, BT_PHY_PKT_NULL, NULL, 0);
--
2.25.1
From: Tedd Ho-Jeong An <[email protected]>
This patch replaces the rand() function to the getrandom() syscall.
It was reported by the Coverity scan
rand() should not be used for security-related applications, because
linear congruential algorithms are too easy to break
---
peripheral/main.c | 11 ++++++-----
1 file changed, 6 insertions(+), 5 deletions(-)
diff --git a/peripheral/main.c b/peripheral/main.c
index 86b52236e..0f5210403 100644
--- a/peripheral/main.c
+++ b/peripheral/main.c
@@ -25,6 +25,7 @@
#include <sys/stat.h>
#include <sys/types.h>
#include <sys/mount.h>
+#include <sys/random.h>
#ifndef WAIT_ANY
#define WAIT_ANY (-1)
@@ -191,11 +192,11 @@ int main(int argc, char *argv[])
addr, 6) < 0) {
printf("Generating new persistent static address\n");
- addr[0] = rand();
- addr[1] = rand();
- addr[2] = rand();
- addr[3] = 0x34;
- addr[4] = 0x12;
+ if (getrandom(addr, sizeof(addr), 0) < 0) {
+ perror("Failed to get random static address");
+ return EXIT_FAILURE;
+ }
+ /* Overwrite the MSB to make it a static address */
addr[5] = 0xc0;
efivars_write("BluetoothStaticAddress",
--
2.25.1
From: Tedd Ho-Jeong An <[email protected]>
This patch replaces the rand() function to the getrandom() syscall.
It was reported by the Coverity scan
rand() should not be used for security-related applications, because
linear congruential algorithms are too easy to break
---
tools/btgatt-server.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)
diff --git a/tools/btgatt-server.c b/tools/btgatt-server.c
index 000145a3d..15d49a464 100644
--- a/tools/btgatt-server.c
+++ b/tools/btgatt-server.c
@@ -20,6 +20,7 @@
#include <getopt.h>
#include <unistd.h>
#include <errno.h>
+#include <sys/random.h>
#include "lib/bluetooth.h"
#include "lib/hci.h"
@@ -284,9 +285,13 @@ static bool hr_msrmt_cb(void *user_data)
uint16_t len = 2;
uint8_t pdu[4];
uint32_t cur_ee;
+ uint32_t val;
+
+ if (getrandom(&val, sizeof(val), 0) < 0)
+ return false;
pdu[0] = 0x06;
- pdu[1] = 90 + (rand() % 40);
+ pdu[1] = 90 + (val % 40);
if (expended_present) {
pdu[0] |= 0x08;
--
2.25.1
From: Tedd Ho-Jeong An <[email protected]>
This patch replaces the rand() function to the getrandom() syscall.
It was reported by the Coverity scan
rand() should not be used for security-related applications, because
linear congruential algorithms are too easy to break
---
plugins/autopair.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/plugins/autopair.c b/plugins/autopair.c
index 665a4f4a6..a75ecebe4 100644
--- a/plugins/autopair.c
+++ b/plugins/autopair.c
@@ -17,6 +17,7 @@
#include <fcntl.h>
#include <unistd.h>
#include <errno.h>
+#include <sys/random.h>
#include <glib.h>
@@ -49,6 +50,7 @@ static ssize_t autopair_pincb(struct btd_adapter *adapter,
char pinstr[7];
char name[25];
uint32_t class;
+ uint32_t val;
ba2str(device_get_address(device), addr);
@@ -129,8 +131,12 @@ static ssize_t autopair_pincb(struct btd_adapter *adapter,
if (attempt >= 4)
return 0;
+ if (getrandom(&val, sizeof(val), 0) < 0) {
+ error("Failed to get a random pincode");
+ return 0;
+ }
snprintf(pinstr, sizeof(pinstr), "%06u",
- rand() % 1000000);
+ val % 1000000);
*display = true;
memcpy(pinbuf, pinstr, 6);
return 6;
--
2.25.1
From: Tedd Ho-Jeong An <[email protected]>
This patch replaces the rand() function to the getrandom() syscall.
It was reported by the Coverity scan
rand() should not be used for security-related applications, because
linear congruential algorithms are too easy to break
---
profiles/health/hdp.c | 11 +++++++----
profiles/health/mcap.c | 17 +++++++++++++++--
2 files changed, 22 insertions(+), 6 deletions(-)
diff --git a/profiles/health/hdp.c b/profiles/health/hdp.c
index 6bc41946f..40b6cc18a 100644
--- a/profiles/health/hdp.c
+++ b/profiles/health/hdp.c
@@ -16,6 +16,7 @@
#include <stdint.h>
#include <stdbool.h>
#include <unistd.h>
+#include <sys/random.h>
#include <glib.h>
@@ -1484,13 +1485,15 @@ static void destroy_create_dc_data(gpointer data)
static void *generate_echo_packet(void)
{
uint8_t *buf;
- int i;
buf = g_malloc(HDP_ECHO_LEN);
- srand(time(NULL));
+ if (!buf)
+ return NULL;
- for(i = 0; i < HDP_ECHO_LEN; i++)
- buf[i] = rand() % UINT8_MAX;
+ if (getrandom(buf, HDP_ECHO_LEN, 0) < 0) {
+ g_free(buf);
+ return NULL;
+ }
return buf;
}
diff --git a/profiles/health/mcap.c b/profiles/health/mcap.c
index 5161ef77c..aad0a08a3 100644
--- a/profiles/health/mcap.c
+++ b/profiles/health/mcap.c
@@ -19,6 +19,7 @@
#include <errno.h>
#include <unistd.h>
#include <time.h>
+#include <sys/random.h>
#include <glib.h>
@@ -1888,6 +1889,7 @@ gboolean mcap_create_mcl(struct mcap_instance *mi,
{
struct mcap_mcl *mcl;
struct connect_mcl *con;
+ uint16_t val;
mcl = find_mcl(mi->mcls, addr);
if (mcl) {
@@ -1903,7 +1905,12 @@ gboolean mcap_create_mcl(struct mcap_instance *mi,
mcl->state = MCL_IDLE;
bacpy(&mcl->addr, addr);
set_default_cb(mcl);
- mcl->next_mdl = (rand() % MCAP_MDLID_FINAL) + 1;
+ if (getrandom(&val, sizeof(val), 0) < 0) {
+ mcap_instance_unref(mcl->mi);
+ g_free(mcl);
+ return FALSE;
+ }
+ mcl->next_mdl = (val % MCAP_MDLID_FINAL) + 1;
}
mcl->ctrl |= MCAP_CTRL_CONN;
@@ -2013,6 +2020,7 @@ static void connect_mcl_event_cb(GIOChannel *chan, GError *gerr,
bdaddr_t dst;
char address[18], srcstr[18];
GError *err = NULL;
+ uint16_t val;
if (gerr)
return;
@@ -2041,7 +2049,12 @@ static void connect_mcl_event_cb(GIOChannel *chan, GError *gerr,
mcl->mi = mcap_instance_ref(mi);
bacpy(&mcl->addr, &dst);
set_default_cb(mcl);
- mcl->next_mdl = (rand() % MCAP_MDLID_FINAL) + 1;
+ if (getrandom(&val, sizeof(val), 0) < 0) {
+ mcap_instance_unref(mcl->mi);
+ g_free(mcl);
+ goto drop;
+ }
+ mcl->next_mdl = (val % MCAP_MDLID_FINAL) + 1;
}
set_mcl_conf(chan, mcl);
--
2.25.1
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=592617
---Test result---
Test Summary:
CheckPatch PASS 6.87 seconds
GitLint PASS 4.68 seconds
Prep - Setup ELL PASS 42.33 seconds
Build - Prep PASS 0.59 seconds
Build - Configure PASS 8.11 seconds
Build - Make PASS 179.48 seconds
Make Check PASS 8.97 seconds
Make Distcheck PASS 204.63 seconds
Build w/ext ELL - Configure PASS 7.72 seconds
Build w/ext ELL - Make PASS 169.93 seconds
Incremental Build with patchesPASS 955.83 seconds
---
Regards,
Linux Bluetooth
Hi Tedd,
On Wed, Dec 8, 2021 at 5:29 PM Tedd Ho-Jeong An <[email protected]> wrote:
>
> From: Tedd Ho-Jeong An <[email protected]>
>
> The Coverity scan reported (CWE-676):
> rand() should not be used for security-related applications, because
> linear congruential algorithms are too easy to break.
>
> This series of patch replaces the standard random number generation
> function, rand(), to getrandom() syscall, which provides more secure
> random number than the standard rand() function.
>
> Tedd Ho-Jeong An (5):
> emulator: Replace random number generation function
> peripheral: Replace random number generation function
> tools/btgatt-server: Replace random number generation function
> plugins: Replace random number generation function
> profiles/health: Replace random number generation function
>
> emulator/le.c | 11 +++++++++--
> emulator/phy.c | 10 ++++++++--
> peripheral/main.c | 11 ++++++-----
> plugins/autopair.c | 8 +++++++-
> profiles/health/hdp.c | 11 +++++++----
> profiles/health/mcap.c | 17 +++++++++++++++--
> tools/btgatt-server.c | 7 ++++++-
> 7 files changed, 58 insertions(+), 17 deletions(-)
>
> --
> 2.25.1
Applied, thanks.
--
Luiz Augusto von Dentz