| Message ID | 20240709120031.105038-4-r.smirnov@omp.ru (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | fix errors found by SVACE static analyzer #3 | expand |
| Context | Check | Description |
|---|---|---|
| tedd_an/pre-ci_am | success | Success |
| tedd_an/CheckPatch | success | CheckPatch PASS |
| tedd_an/GitLint | success | Gitlint PASS |
| tedd_an/IncrementalBuild | success | Incremental Build PASS |
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 > >
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) { + 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; }