From 4db8535e3950ad5d762876ac1f4a02b9e96a026d Mon Sep 17 00:00:00 2001 From: Jef Driesen Date: Sun, 4 May 2014 22:43:54 +0200 Subject: [PATCH] Use the extended sample length stored in the data. Currently, the buffer overflow checks take into account the size of the entire dive. But since the length of the extended sample is stored in the data, we can actually check for overflows in each sample. The main benefit is that errors will be caught much earlier now. An additional advantage is that we can now easily skip any remaining sample bytes. Normally such bytes are not present, unless a firmware update introduces a new feature which our parser doesn't support yet. --- src/hw_ostc_parser.c | 27 ++++++++++++++++++++------- 1 file changed, 20 insertions(+), 7 deletions(-) diff --git a/src/hw_ostc_parser.c b/src/hw_ostc_parser.c index c090faa..26fea22 100644 --- a/src/hw_ostc_parser.c +++ b/src/hw_ostc_parser.c @@ -486,13 +486,14 @@ hw_ostc_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t call while (data[offset - 1] & 0x80) { if (nbits && version != 0x23) break; - if (offset + 1 > size) { + if (length < 1) { ERROR (abstract->context, "Buffer overflow detected!"); return DC_STATUS_DATAFORMAT; } events |= data[offset] << nbits; nbits += 8; offset++; + length--; } // Alarms @@ -529,7 +530,7 @@ hw_ostc_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t call // Manual Gas Set & Change if (events & 0x10) { - if (offset + 2 > size) { + if (length < 2) { ERROR (abstract->context, "Buffer overflow detected!"); return DC_STATUS_DATAFORMAT; } @@ -539,11 +540,12 @@ hw_ostc_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t call sample.event.value = data[offset] | (data[offset + 1] << 16); if (callback) callback (DC_SAMPLE_EVENT, sample, userdata); offset += 2; + length -= 2; } // Gas Change if (events & 0x20) { - if (offset + 1 > size) { + if (length < 1) { ERROR (abstract->context, "Buffer overflow detected!"); return DC_STATUS_DATAFORMAT; } @@ -557,23 +559,25 @@ hw_ostc_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t call sample.event.value = gasmix[idx].oxygen | (gasmix[idx].helium << 16); if (callback) callback (DC_SAMPLE_EVENT, sample, userdata); offset++; + length--; } // SetPoint Change if ((events & 0x40) && (version == 0x23)) { - if (offset + 1 > size) { + if (length < 1) { ERROR (abstract->context, "Buffer overflow detected!"); return DC_STATUS_DATAFORMAT; } sample.setpoint = data[offset] / 100.0; if (callback) callback (DC_SAMPLE_SETPOINT, sample, userdata); offset++; + length--; } // Extended sample info. for (unsigned int i = 0; i < nconfig; ++i) { if (info[i].divisor && (nsamples % info[i].divisor) == 0) { - if (offset + info[i].size > size) { + if (length < info[i].size) { ERROR (abstract->context, "Buffer overflow detected!"); return DC_STATUS_DATAFORMAT; } @@ -608,24 +612,26 @@ hw_ostc_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t call } offset += info[i].size; + length -= info[i].size; } } if (version != 0x23) { // SetPoint Change if (events & 0x40) { - if (offset + 1 > size) { + if (length < 1) { ERROR (abstract->context, "Buffer overflow detected!"); return DC_STATUS_DATAFORMAT; } sample.setpoint = data[offset] / 100.0; if (callback) callback (DC_SAMPLE_SETPOINT, sample, userdata); offset++; + length--; } // Bailout Event if (events & 0x80) { - if (offset + 2 > size) { + if (length < 2) { ERROR (abstract->context, "Buffer overflow detected!"); return DC_STATUS_DATAFORMAT; } @@ -635,8 +641,15 @@ hw_ostc_parser_samples_foreach (dc_parser_t *abstract, dc_sample_callback_t call sample.event.value = data[offset] | (data[offset + 1] << 16); if (callback) callback (DC_SAMPLE_EVENT, sample, userdata); offset += 2; + length -= 2; } } + + // Skip remaining sample bytes (if any). + if (length) { + WARNING (abstract->context, "Remaining %u bytes skipped.", length); + } + offset += length; } if (offset + 2 > size || data[offset] != 0xFD || data[offset + 1] != 0xFD) {