Merge pull request #168 from ladyada-eagleclaw/fix-truncated-sentences

Reject truncated NMEA sentences before parsing fields

extras/tests/truncated_sentence/truncated_sentence.ino

@@ -0,0 +1,48 @@
+#include <Adafruit_GPS.h>
+
+Adafruit_GPS GPS;
+bool testsPassed = false;
+
+void setup() {
+  Serial.begin(115200);
+  // Wait briefly for the Serial Monitor on native USB boards.
+  while (!Serial && millis() < 5000) {
+    delay(10);
+  }
+  delay(250);
+  Serial.println("Adafruit GPS truncated sentence regression");
+
+  char good[] = "$GPRMC,123519,A,4807.038,N,01131.000,E,022.4,084.4,230394,003.1,W*6A";
+  if (!GPS.parse(good)) {
+    Serial.println("FAIL: valid RMC sentence was rejected");
+    return;
+  }
+  Serial.println("PASS: valid RMC sentence was accepted");
+  nmea_float_t latitude = GPS.latitude;
+  nmea_float_t longitude = GPS.longitude;
+
+  // This checksum is valid, but the sentence from issue #128 is incomplete.
+  char bad[] = "$GPRMC,181536.000,A,5936.79K,D*3A";
+  if (GPS.parse(bad)) {
+    Serial.println("FAIL: truncated RMC sentence was accepted");
+    return;
+  }
+  Serial.println("PASS: truncated RMC sentence was rejected");
+  if (GPS.hour != 12 || GPS.minute != 35 || GPS.seconds != 19 ||
+      GPS.day != 23 || GPS.month != 3 || GPS.year != 94 ||
+      GPS.latitude != latitude || GPS.longitude != longitude) {
+    Serial.println("FAIL: truncated sentence changed the last GPS data");
+    return;
+  }
+  Serial.println("PASS: time, date, and position were preserved");
+  testsPassed = true;
+}
+
+void loop() {
+  if (testsPassed) {
+    Serial.println("PASS: truncated sentence regression");
+  } else {
+    Serial.println("FAIL: truncated sentence regression");
+  }
+  delay(2000);
+}

src/NMEA_parse.cpp

@@ -52,6 +52,17 @@
 bool Adafruit_GPS::parse(char *nmea) {
   if (!check(nmea))
     return false;
+  // Count fields before parsing so a truncated sentence cannot change GPS data
+  // or make a comma lookup run past the end of the sentence.
+  size_t fields = 0;
+  for (char *p = nmea; *p && *p != '*'; p++) {
+    if (*p == ',')
+      fields++;
+  }
+  // All current sentence handlers require fields. Revisit this guard if a
+  // zero-field sentence is added; the comma lookup below also assumes a field.
+  if (fields == 0)
+    return false;
   // passed the check, so there's a valid source in thisSource and a valid
   // sentence in thisSentence
   char *p = nmea; // Pointer to move through the sentence -- good parsers are
@@ -63,6 +74,8 @@ bool Adafruit_GPS::parse(char *nmea) {
   // pruning excess code easier. Otherwise, keep them alphabetical for ease of
   // reading.
   if (!strcmp(thisSentence, "GGA")) { //************************************GGA
+    if (fields < 11)
+      return false;
     // Adafruit from Actisense NGW-1 from SH CP150C
     parseTime(p);
     p = strchr(p, ',') + 1; // parse time with specialized function
@@ -101,6 +114,8 @@ bool Adafruit_GPS::parse(char *nmea) {
       geoidheight = atof(p); // skip the rest
 
   } else if (!strcmp(thisSentence, "RMC")) { //*****************************RMC
+    if (fields < 9)
+      return false;
     // in Adafruit from Actisense NGW-1 from SH CP150C
     parseTime(p);
     p = strchr(p, ',') + 1;
@@ -131,6 +146,8 @@ bool Adafruit_GPS::parse(char *nmea) {
     } // skip the rest
 
   } else if (!strcmp(thisSentence, "GLL")) { //*****************************GLL
+    if (fields < 6)
+      return false;
     // in Adafruit from Actisense NGW-1 from SH CP150C
     // parse out both latitude and direction, then go to next field, or fail
     if (parseCoord(p, &latitudeDegrees, &latitude, &latitude_fixed, &lat))
@@ -147,6 +164,8 @@ bool Adafruit_GPS::parse(char *nmea) {
     parseFix(p); // skip the rest
 
   } else if (!strcmp(thisSentence, "GSA")) { //*****************************GSA
+    if (fields < 17)
+      return false;
     // in Adafruit from Actisense NGW-1
     p = strchr(p, ',') + 1; // skip selection mode
     if (!isEmpty(p))
@@ -167,6 +186,8 @@ bool Adafruit_GPS::parse(char *nmea) {
       VDOP = atof(p); // last before checksum
 
   } else if (!strcmp(thisSentence, "TOP")) { //*****************************TOP
+    if (fields < 2)
+      return false;
     // See:
     // https://learn.adafruit.com/adafruit-ultimate-gps-featherwing/antenna-options
     // There is an output sentence that will tell you the status of the
@@ -182,6 +203,8 @@ bool Adafruit_GPS::parse(char *nmea) {
     return false;
 
   } else if (!strcmp(thisSentence, "DBT")) { //*****************************DBT
+    if (fields < 5)
+      return false;
     // from Actisense NGW-1
     // feet, metres, fathoms below transducer coerced to water depth from
     // surface in metres
@@ -219,6 +242,8 @@ bool Adafruit_GPS::parse(char *nmea) {
       newDataValue(NMEA_HDT, atof(p)); // skip the rest
 
   } else if (!strcmp(thisSentence, "MDA")) { //*****************************MDA
+    if (fields < 9)
+      return false;
     // from Actisense NGW-1
     if (!isEmpty(p))
       newDataValue(NMEA_BAROMETER, atof(p) * 3386.39);
@@ -260,6 +285,8 @@ bool Adafruit_GPS::parse(char *nmea) {
       newDataValue(NMEA_HUMIDITY, atof(p)); // skip the rest
 
   } else if (!strcmp(thisSentence, "MTW")) { //*****************************MTW
+    if (fields < 2)
+      return false;
     nmea_float_t T = 100000.;
     char u = 'C';
     if (!isEmpty(p))
@@ -279,6 +306,8 @@ bool Adafruit_GPS::parse(char *nmea) {
     return false;
 
   } else if (!strcmp(thisSentence, "MWV")) { //*****************************MWV
+    if (fields < 5)
+      return false;
     // from Actisense NGW-1
     nmea_float_t ang = 100000.;
     char ref = 'T';
@@ -322,6 +351,8 @@ bool Adafruit_GPS::parse(char *nmea) {
     }
 
   } else if (!strcmp(thisSentence, "RMB")) { //*****************************RMB
+    if (fields < 12)
+      return false;
     // from Actisense NGW-1 from SH CP150C
     // RMB Recommended Minimum Navigation Information
     //       1 2   3 4    5    6       7 8        9 10  11 12  13 14
@@ -409,6 +440,8 @@ bool Adafruit_GPS::parse(char *nmea) {
     return false;
 
   } else if (!strcmp(thisSentence, "TXT")) { //*****************************TXT
+    if (fields < 4)
+      return false;
     if (!isEmpty(p))
       txtTot = atoi(p);
     p = strchr(p, ',') + 1;
@@ -426,6 +459,8 @@ bool Adafruit_GPS::parse(char *nmea) {
     return false;
 
   } else if (!strcmp(thisSentence, "VHW")) { //*****************************VHW
+    if (fields < 5)
+      return false;
     // from Actisense NGW-1
     if (!isEmpty(p))
       newDataValue(NMEA_HDT, atof(p));
@@ -439,6 +474,8 @@ bool Adafruit_GPS::parse(char *nmea) {
       newDataValue(NMEA_VTW, atof(p)); // skip the other units
 
   } else if (!strcmp(thisSentence, "VLW")) { //*****************************VLW
+    if (fields < 3)
+      return false;
     // from Actisense NGW-1
     if (!isEmpty(p))
       newDataValue(NMEA_LOG, atof(p));
@@ -448,6 +485,8 @@ bool Adafruit_GPS::parse(char *nmea) {
       newDataValue(NMEA_LOGR, atof(p)); // skip the other units
 
   } else if (!strcmp(thisSentence, "VPW")) { //*****************************VPW
+    if (fields < 3)
+      return false;
     // knots, metres/s coerced to knots
     nmea_float_t vmg = 100000.;
     if (!isEmpty(p))
@@ -463,6 +502,8 @@ bool Adafruit_GPS::parse(char *nmea) {
     return false;
 
   } else if (!strcmp(thisSentence, "VWR")) { //*****************************VWR
+    if (fields < 8)
+      return false;
     // from Actisense NGW-1
     nmea_float_t ang = 1000.;
     if (!isEmpty(p))
@@ -516,6 +557,8 @@ bool Adafruit_GPS::parse(char *nmea) {
       newDataValue(NMEA_VMGWP, atof(p)); // skip the rest
 
   } else if (!strcmp(thisSentence, "XTE")) { //*****************************XTE
+    if (fields < 5)
+      return false;
     // from Actisense NGW-1 from SH CP150C
     p = strchr(p, ',') + 1; // skip status 1
     p = strchr(p, ',') + 1; // skip status 2
@@ -695,7 +738,8 @@ bool Adafruit_GPS::parseCoord(char *pStart, nmea_float_t *angleDegrees,
     // get the number in DDDMM.mmmm format and break into components
     char degreebuff[10] = {0}; // Ensure string is terminated after strncpy
     char *e = strchr(p, '.');
-    if (e == NULL || e - p > 6)
+    char *comma = strchr(p, ',');
+    if (e == NULL || comma == NULL || e > comma || e - p > 6)
       return false;                // no decimal point in range
     strncpy(degreebuff, p, e - p); // get DDDMM
     long dddmm = atol(degreebuff);
@@ -703,7 +747,7 @@ bool Adafruit_GPS::parseCoord(char *pStart, nmea_float_t *angleDegrees,
     long minutes = dddmm - degrees * 100; // remove the degrees
     p = e;                                // start from the decimal point
     nmea_float_t decminutes = atof(e); // the fraction after the decimal point
-    p = strchr(p, ',') + 1;            // go to the next field
+    p = comma + 1;                     // go to the direction field
 
     // get the NSEW direction as a character
     char nsew = 'X';