From 89a58e23e095c50dc844cbbed577c989cf801e32 Mon Sep 17 00:00:00 2001 From: Dirk Hohndel Date: Thu, 12 Dec 2013 16:12:04 -0800 Subject: [PATCH] Correctly parse multiple tags in the divelog The old parsing code overwrote the first comma with a '\0' and then checked the index against the length of the buffer - which was changed by replacing the ',' with the '\0'. This means that since commit 78acf20848c0 ("Don't crash on loading tags longer than 127 chars") Subsurface has potentially damaged / lost data in dive files! Added a test dive that shows the issue if opened by a Subsurface version after the commit mentioned above but before this commit. Reported-by: Miika Turkia Signed-off-by: Dirk Hohndel --- dives/test29.xml | 62 ++++++++++++++++++++++++++++++++++++++++++++++++ parse-xml.c | 22 ++++++++--------- 2 files changed, 72 insertions(+), 12 deletions(-) create mode 100644 dives/test29.xml diff --git a/dives/test29.xml b/dives/test29.xml new file mode 100644 index 000000000..9fb40a61d --- /dev/null +++ b/dives/test29.xml @@ -0,0 +1,62 @@ + + + + + + irrelevant dive location + This is a dive that has two tags, boat and wreck + + + + + + + + + + + + + irrelevant dive location + This is a dive that has an empty tags property in the XML file + + + + + + + + + + + + + irrelevant dive location + This is a dive that has a tags property with just a comma in the XML file + + + + + + + + + + + + + irrelevant dive location + This is a dive that has a single letter tag 'a' (plus the boat tag) + + + + + + + + + + + + + diff --git a/parse-xml.c b/parse-xml.c index 9287e0815..3a3f65419 100644 --- a/parse-xml.c +++ b/parse-xml.c @@ -220,19 +220,18 @@ static void divetags(char *buffer, void *_tags) struct tag_entry *tags = _tags; int i = 0, start = 0, end = 0; enum ParseState state = FINDEND; - i=0; - while(i < strlen(buffer)) { + int len = buffer ? strlen(buffer) : 0; + + while(i < len) { if (buffer[i] == ',') { if (state == FINDSTART) { /* Detect empty tags */ } else if (state == FINDEND) { /* Found end of tag */ - if (i > 1) { - if(buffer[i-1] != '\\') { - buffer[end-start+1] = '\0'; + if (i > 0 && buffer[i - 1] != '\\') { + buffer[i] = '\0'; state=FINDSTART; taglist_add_tag(tags, buffer+start); - } } else { state=FINDSTART; } @@ -245,18 +244,17 @@ static void divetags(char *buffer, void *_tags) state = FINDEND; start = i; } else if (state == FINDEND) { - end=i; + end = i; } } i++; } if (state == FINDEND) { if (end < start) - end = strlen(buffer)-1; - if (strlen(buffer) > 0) { - buffer[end-start+1] = '\0'; - state=FINDSTART; - taglist_add_tag(tags, buffer+start); + end = len - 1; + if (len > 0) { + buffer[end + 1] = '\0'; + taglist_add_tag(tags, buffer + start); } } }