From 064e198315715e4e03b404e23f5251734fdf283b Mon Sep 17 00:00:00 2001 From: Linus Torvalds Date: Mon, 2 Jan 2023 14:23:27 -0800 Subject: [PATCH] garmin: relax string parsing sanity checks MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Signed-off-by: Linus Torvalds --- src/garmin_parser.c | 19 ++++--------------- 1 file changed, 4 insertions(+), 15 deletions(-) diff --git a/src/garmin_parser.c b/src/garmin_parser.c index 8f53a93..df73aea 100644 --- a/src/garmin_parser.c +++ b/src/garmin_parser.c @@ -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) {