Hi Roman,
On Tue, Jul 9, 2024 at 8:01 AM Roman Smirnov <r.smirnov@xxxxxx> 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
>
>
--
Luiz Augusto von Dentz
