FlapJackApps/libdc
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

garmin: relax string parsing sanity checks

Browse Source 
The garmin FIT file parser verified that a string entry fit in the field
size, but it turns out that the check is wrong: a FIT file string field
is not necessarily NUL-terminated at all, and it's ok to have a string
that fills the entire field.

We never actually then use the string length we just checked, so with
the checks being bogus, all of this code just goes away.  But let's
update the debug printout to follow these rules.

This makes parsing the example FIT file that Cédric sent us work just
fine (at least superficially, in that I don't see anything obviously
wrong with the result: I don't actually know what Cédric's dive was
supposed to look like to verify).

Reported-by: Cédric BAREYT <bareytcedric@gmail.com>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
This commit is contained in:
Linus Torvalds 2023-01-02 14:23:27 -08:00
parent 6200a7923f
commit 064e198315
1 changed files with 4 additions and 15 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

19
src/garmin_parser.c
View File

@ -1170,9 +1170,12 @@ static void unknown_field(struct garmin_parser_t *garmin, const unsigned char *d
data += base_size;
len -= base_size;
}
/* Avoid debug printing limit */
len = sizeof(buffer);
}
DEBUG(garmin->base.context, "%s/%d %s '%s'", msg_name, field_nr, base_type_info[base_type].type_name, str);
DEBUG(garmin->base.context, "%s/%d %s '%.*s'", msg_name, field_nr, base_type_info[base_type].type_name, len, str);
}
@ -1220,20 +1223,6 @@ static int traverse_regular(struct garmin_parser_t *garmin,
ERROR(garmin->base.context, "Data traversal size not a multiple of base size (%d vs %d)\n", len, base_size);
return -1;
}
// String
if (base_type == 7) {
int string_len = strnlen(data, size);
if (string_len >= size) {
ERROR(garmin->base.context, "Data traversal string bigger than remaining data\n");
return -1;
}
if (len <= string_len) {
ERROR(garmin->base.context, "field length %d, string length %d\n", len, string_len + 1);
return -1;
}
}
// Certain field numbers have fixed meaning across all messages
switch (field_nr) {