mbox series

[BlueZ,v1,0/4] fix errors found by SVACE static analyzer #3

Message ID 20240709120031.105038-1-r.smirnov@omp.ru
Headers show
Series fix errors found by SVACE static analyzer #3 | expand

Message

Roman Smirnov July 9, 2024, noon UTC 
Several bug fixes.

Roman Smirnov (4):
  health: mcap: add checks for NULL mcap_notify_error()
  shared: prevent dereferencing of NULL pointers
  settings: limit the string size in load_service()
  settings: limit the number of chars to be read in gatt_db_load()

 profiles/health/mcap.c |  9 +++++++
 src/settings.c         | 58 +++++++++++++++++++++++++++++++++++++++---
 src/shared/micp.c      |  4 +++
 src/shared/vcp.c       | 12 +++++++++
 4 files changed, 80 insertions(+), 3 deletions(-)

Comments

Luiz Augusto von Dentz July 9, 2024, 2:02 p.m. UTC | #1 
Hi Roman,

On Tue, Jul 9, 2024 at 8:01 AM Roman Smirnov <r.smirnov@omp.ru> wrote:
>
> Calculate the length of the first string and use it to create
> a pattern. The pattern will limit the maximum length of the
> string, which will prevent the buffer from overflowing.
>
> Found with the SVACE static analysis tool.
> ---
>  src/settings.c | 19 ++++++++++++++++++-
>  1 file changed, 18 insertions(+), 1 deletion(-)
>
> diff --git a/src/settings.c b/src/settings.c
> index b61e694f1..4eccf0b4e 100644
> --- a/src/settings.c
> +++ b/src/settings.c
> @@ -187,13 +187,30 @@ static int load_service(struct gatt_db *db, char *handle, char *value)
>         char type[MAX_LEN_UUID_STR], uuid_str[MAX_LEN_UUID_STR];
>         bt_uuid_t uuid;
>         bool primary;
> +       char pattern[16];
> +       char *colon_pos;
> +       size_t len;
>
>         if (sscanf(handle, "%04hx", &start) != 1) {
>                 DBG("Failed to parse handle: %s", handle);
>                 return -EIO;
>         }
>
> -       if (sscanf(value, "%[^:]:%04hx:%36s", type, &end, uuid_str) != 3) {

Can't we just do %36[^:] instead since it is the same size of
uuid_str, the only real difference is that it reads until the ':'
rather than until the end, but %36s is also _at most_ 36 characters.

> +       colon_pos = memchr(value, ':', MAX_LEN_UUID_STR);
> +       if (!colon_pos) {
> +               DBG("Failed to parse value: %s", value);
> +               return -EIO;
> +       }
> +
> +       len = colon_pos - value;
> +       if (!len) {
> +               DBG("Failed to parse value: %s", value);
> +               return -EIO;
> +       }
> +
> +       snprintf(pattern, sizeof(pattern), "%%%lds:%%04hx:%%36s", len);
> +
> +       if (sscanf(value, pattern, type, &end, uuid_str) != 3) {
>                 DBG("Failed to parse value: %s", value);
>                 return -EIO;
>         }
> --
> 2.34.1
>
>