gps_read(): fix some nasty buffer overruns and corruptions.

Now pass an optional message buffer to gps_read().  Finally
the JSON display in cgps works.

Thanks to Virgin Orbit for their support fixing this bug.

9 files changed, 80 insertions, 53 deletions

diff --git a/cgps.c b/cgps.c
index 7790d134..85de2dfa 100644
--- a/cgps.c
+++ b/cgps.c
@@ -101,6 +101,7 @@
 
 #include "gpsd_config.h"
 #include "gps.h"
+#include "gps_json.h"   /* for GPS_JSON_RESPONSE_MAX */
 #include "compiler.h"	/* for UNUSED */
 #include "gpsdclient.h"
 #include "revision.h"
@@ -456,7 +457,7 @@ static int sat_cmp(const void *p1, const void *p2)
 }
 
 
-static void update_gps_panel(struct gps_data_t *gpsdata)
+static void update_gps_panel(struct gps_data_t *gpsdata, char * message)
 /* This gets called once for each new GPS sentence. */
 {
     int newstate;
@@ -711,23 +712,19 @@ static void update_gps_panel(struct gps_data_t *gpsdata)
 
     }
 
-    if ( raw_flag) {
-        /* Be quiet if the user requests silence. */
-        const char *sr;
-        if (!silent_flag && (sr = gps_data(gpsdata)) != NULL) {
-            char *p, *pe;
-
-            /* make a copy of the const char *, then trim trailing spaces */
-            p = strdup(sr);
-            if ( NULL != p ) {
-		pe = p + strlen(p);
-		for ( ; --pe > p && isspace((int) *pe); *pe = '\0')
-		    ;
-		(void)wprintw(messages, "%s\n", p);
-		free(p);
+    /* Be quiet if the user requests silence. */
+    if (!silent_flag && raw_flag) {
+        if (NULL != message) {
+            size_t message_len = strlen(message);
+            if (0 < message_len) {
+                if ( '\r' == message[message_len - 1]) {
+                    /* remove any trailing \r */
+                    message[message_len - 1] = '\0';
+                }
+                (void)wprintw(messages, "\n%s", message);
+                (void)wrefresh(messages);
             }
         }
-        (void)wrefresh(messages);
     }
 
     /* Reset the status_timer if the state has changed. */
@@ -767,6 +764,8 @@ int main(int argc, char *argv[])
     int option;
     unsigned int flags = WATCH_ENABLE;
     int wait_clicks = 0;  /* cycles to wait before gpsd timeout */
+    /* buffer to hold one JSON message */
+    char message[GPS_JSON_RESPONSE_MAX];
 
     switch (gpsd_units()) {
     case imperial:
@@ -896,7 +895,8 @@ int main(int argc, char *argv[])
 	} else {
 	    wait_clicks = 0;
 	    errno = 0;
-	    if (gps_read(&gpsdata) == -1) {
+            *message = '\0';
+           if (gps_read(&gpsdata, message, sizeof(message)) == -1) {
 		(void)fprintf(stderr, "cgps: socket error 4\n");
 		die(errno == 0 ? GPS_GONE : GPS_ERROR);
 	    } else {
@@ -906,7 +906,7 @@ int main(int argc, char *argv[])
 		    update_compass_panel(&gpsdata);
 		else
 #endif /* TRUENORTH */
-		    update_gps_panel(&gpsdata);
+		    update_gps_panel(&gpsdata, message);
 	    }
 	}
 
diff --git a/gps.h b/gps.h
index f320e9ee..c5007452 100644
--- a/gps.h
+++ b/gps.h
@@ -37,6 +37,7 @@ extern "C" {
  *       bits less confusing. (January 2015, release 3.12).
  * 6.1 - Add navdata_t for more (nmea2000) info.
  * 7.0 - add gps_fix_t.ecef (February 2018)
+ *       changed prototype of gps_read() to add buffer parameters
  */
 #define GPSD_API_MAJOR_VERSION	7	/* bump on incompatible changes */
 #define GPSD_API_MINOR_VERSION	0	/* bump on compatible changes */
@@ -1816,11 +1817,11 @@ struct ais_t
 
 /* basic data, per PRN, from GPGSA and GPGSV */
 struct satellite_t {
-    double ss;		/* signal-to-noise ratio (dB) */
+    double ss;		/* signal-to-noise ratio, 0 to 254 dB, -1 for n/a */
     bool used;		/* this satellite used in solution */
-    short PRN;		/* PRN of this satellite */
-    short elevation;	/* elevation of satellite */
-    short azimuth;	/* azimuth */
+    short PRN;		/* PRN of this satellite, 1 to 437, 0 for n/a */
+    short elevation;	/* elevation of satellite, -90 to 90 deg, -91 for n/a */
+    short azimuth;	/* azimuth, 0 to 359 deg, -1 for n/a */
 };
 
 struct attitude_t {
@@ -2089,7 +2090,7 @@ extern int gps_open(const char *, const char *,
 		      struct gps_data_t *);
 extern int gps_close(struct gps_data_t *);
 extern int gps_send(struct gps_data_t *, const char *, ... );
-extern int gps_read(struct gps_data_t *);
+extern int gps_read(struct gps_data_t *, char *message, int message_len);
 extern int gps_unpack(char *, struct gps_data_t *);
 extern bool gps_waiting(const struct gps_data_t *, int);
 extern int gps_stream(struct gps_data_t *, unsigned int, void *);
diff --git a/gpsctl.c b/gpsctl.c
index 9a425f16..b38143ac 100644
--- a/gpsctl.c
+++ b/gpsctl.c
@@ -127,7 +127,7 @@ static bool gps_query(struct gps_data_t *gpsdata,
 
 	gpsd_log(&context.errout, LOG_PROG, "reading...\n");
 
-	(void)gps_read(gpsdata);
+	(void)gps_read(gpsdata, NULL, 0);
 	if (ERROR_SET & gpsdata->set) {
 	    gpsd_log(&context.errout, LOG_ERROR, "error '%s'\n", gpsdata->error);
 	    return false;
@@ -437,7 +437,7 @@ int main(int argc, char **argv)
 
 	    while (devcount > 0) {
 		errno = 0;
-		if (gps_read(&gpsdata) == -1) {
+		if (gps_read(&gpsdata, NULL, 0) == -1) {
 		    gpsd_log(&context.errout, LOG_ERROR, "data read failed.\n");
 		    (void)gps_close(&gpsdata);
 		    exit(EXIT_FAILURE);
diff --git a/lcdgps.c b/lcdgps.c
index 52730c43..74874723 100644
--- a/lcdgps.c
+++ b/lcdgps.c
@@ -408,7 +408,7 @@ int main(int argc, char *argv[])
 	    (void)fprintf(stderr, "lcdgps: error while waiting\n");
 	    exit(EXIT_FAILURE);
 	} else {
-	    (void)gps_read(&gpsdata);
+	    (void)gps_read(&gpsdata, NULL, 0);
 	    update_lcd(&gpsdata);
 	}
 
diff --git a/libgps.h b/libgps.h
index 91d2d2c6..62c37423 100644
--- a/libgps.h
+++ b/libgps.h
@@ -21,7 +21,7 @@ extern int gps_sock_open(const char *, const char *,
 		      struct gps_data_t *);
 extern int gps_sock_close(struct gps_data_t *);
 extern int gps_sock_send(struct gps_data_t *, const char *);
-extern int gps_sock_read(struct gps_data_t *);
+extern int gps_sock_read(struct gps_data_t *, char *message, int message_len);
 extern bool gps_sock_waiting(const struct gps_data_t *, int);
 extern int gps_sock_stream(struct gps_data_t *, unsigned int, void *);
 extern const char *gps_sock_data(const struct gps_data_t *);
diff --git a/libgps_core.c b/libgps_core.c
index 29ac6da1..e02ff1b9 100644
--- a/libgps_core.c
+++ b/libgps_core.c
@@ -146,12 +146,18 @@ int gps_close(struct gps_data_t *gpsdata CONDITIONALLY_UNUSED)
 	return status;
 }
 
-int gps_read(struct gps_data_t *gpsdata CONDITIONALLY_UNUSED)
+int gps_read(struct gps_data_t *gpsdata CONDITIONALLY_UNUSED,
+             char *message, int message_len)
 /* read from a gpsd connection */
 {
     int status = -1;
 
     libgps_debug_trace((DEBUG_CALLS, "gps_read() begins\n"));
+    if ((NULL != message) && (0 < message_len)) {
+        /* be sure message is zero length */
+        /* we do not memset() as this is time critical input path */
+        *message = '\0';
+    }
 
 #ifdef SHM_EXPORT_ENABLE
     if (BAD_SOCKET((intptr_t)(gpsdata->gps_fd))) {
@@ -161,7 +167,7 @@ int gps_read(struct gps_data_t *gpsdata CONDITIONALLY_UNUSED)
 
 #ifdef SOCKET_EXPORT_ENABLE
     if (status == -1 && !BAD_SOCKET((intptr_t)(gpsdata->gps_fd))) {
-        status = gps_sock_read(gpsdata);
+        status = gps_sock_read(gpsdata, message, message_len);
     }
 #endif /* SOCKET_EXPORT_ENABLE */
 
diff --git a/libgps_sock.c b/libgps_sock.c
index c4303da5..63635234 100644
--- a/libgps_sock.c
+++ b/libgps_sock.c
@@ -178,23 +178,27 @@ int gps_sock_close(struct gps_data_t *gpsdata)
 #endif
 }
 
-int gps_sock_read(struct gps_data_t *gpsdata)
+int gps_sock_read(struct gps_data_t *gpsdata, char *message, int message_len)
 /* wait for and read data being streamed from the daemon */
 {
     char *eol;
     ssize_t response_length;
     int status = -1;
 
+    errno = 0;
     gpsdata->set &= ~PACKET_SET;
+
+    /* scan to find end of message (\n), or end of buffer */
     for (eol = PRIVATE(gpsdata)->buffer;
-	 *eol != '\n' && eol < PRIVATE(gpsdata)->buffer + PRIVATE(gpsdata)->waiting; eol++)
-	continue;
-    if (*eol != '\n')
-	eol = NULL;
+	 eol < (PRIVATE(gpsdata)->buffer + PRIVATE(gpsdata)->waiting);
+         eol++) {
+        if ('\n' == *eol)
+            break;
+    }
 
-    errno = 0;
+    if (*eol != '\n') {
+	/* no full message found, try to fill buffer */
 
-    if (eol == NULL) {
 #ifndef USE_QT
 	/* read data: return -1 if no data waiting or buffered, 0 otherwise */
 	status = (int)recv(gpsdata->gps_fd,
@@ -210,11 +214,13 @@ int gps_sock_read(struct gps_data_t *gpsdata)
 #ifdef HAVE_WINSOCK2_H
 	int wserr = WSAGetLastError();
 #endif /* HAVE_WINSOCK2_H */
+
 	/* if we just received data from the socket, it's in the buffer */
 	if (status > -1)
 	    PRIVATE(gpsdata)->waiting += status;
-	/* buffer is empty - implies no data was read */
+
 	if (PRIVATE(gpsdata)->waiting == 0) {
+	    /* buffer is empty - implies no data was read */
 	    /*
 	     * If we received 0 bytes, other side of socket is closing.
 	     * Return -1 as end-of-data indication.
@@ -237,23 +243,37 @@ int gps_sock_read(struct gps_data_t *gpsdata)
 	    else
 		return -1;
 	}
-	/* there's buffered data waiting to be returned */
+
+	/* there's new buffered data waiting, check for full message */
 	for (eol = PRIVATE(gpsdata)->buffer;
-	     *eol != '\n' && eol < PRIVATE(gpsdata)->buffer + PRIVATE(gpsdata)->waiting; eol++)
-	    continue;
+	     eol < (PRIVATE(gpsdata)->buffer + PRIVATE(gpsdata)->waiting);
+             eol++) {
+	    if ('\n' == *eol)
+		break;
+	}
 	if (*eol != '\n')
-	    eol = NULL;
-	if (eol == NULL)
+            /* still no full message, give up for now */
 	    return 0;
     }
 
-    assert(eol != NULL);
+    /* eol now points to trailing \n in a full message */
     *eol = '\0';
-    response_length = eol - PRIVATE(gpsdata)->buffer + 1;
+    if (NULL != message) {
+        strlcpy(message, PRIVATE(gpsdata)->buffer, message_len);
+    }
     gpsdata->online = timestamp();
+    /* unpack the JSON message */
     status = gps_unpack(PRIVATE(gpsdata)->buffer, gpsdata);
-    memmove(PRIVATE(gpsdata)->buffer,
-	    PRIVATE(gpsdata)->buffer + response_length, PRIVATE(gpsdata)->waiting - response_length);
+
+    response_length = eol - PRIVATE(gpsdata)->buffer + 1;
+    if (0 == (PRIVATE(gpsdata)->waiting - response_length)) {
+        /* no waiting data, clear the buffer, just in case */
+        *PRIVATE(gpsdata)->buffer = '\0';
+    } else {
+	memmove(PRIVATE(gpsdata)->buffer,
+		PRIVATE(gpsdata)->buffer + response_length,
+		PRIVATE(gpsdata)->waiting - response_length);
+    }
     PRIVATE(gpsdata)->waiting -= response_length;
     gpsdata->set |= PACKET_SET;
 
@@ -292,6 +312,7 @@ int gps_unpack(char *buf, struct gps_data_t *gpsdata)
 const char *gps_sock_data(const struct gps_data_t *gpsdata)
 /* return the contents of the client data buffer */
 {
+    /* no length data, so pretty useless... */
     return PRIVATE(gpsdata)->buffer;
 }
 
@@ -384,7 +405,7 @@ int gps_sock_mainloop(struct gps_data_t *gpsdata, int timeout,
 	if (!gps_waiting(gpsdata, timeout)) {
 	    return -1;
 	} else {
-	    int status = gps_read(gpsdata);
+	    int status = gps_read(gpsdata, NULL, 0);
 
 	    if (status == -1)
 		return -1;
diff --git a/libgpsmm.cpp b/libgpsmm.cpp
index ba4aeb0b..edfd9901 100644
--- a/libgpsmm.cpp
+++ b/libgpsmm.cpp
@@ -1,9 +1,7 @@
 /*
  * Copyright (C) 2005 Alfredo Pironti
  *
- * This software is distributed under a BSD-style license. See the
- * file "COPYING" in the top-level directory of the disribution for details.
- *
+ * SPDX-License-Identifier: BSD-2-clause
  */
 
 #include <cstdlib>
@@ -47,7 +45,7 @@ struct gps_data_t* gpsmm::send(const char *request)
 
 struct gps_data_t* gpsmm::read(void)
 {
-    if (gps_read(gps_state())<=0) {
+    if (gps_read(gps_state(), NULL, 0)<=0) {
 	// we return null if there was a read() error, if no
 	// data was ready in POLL_NOBLOCK mode, or if the
 	// connection is closed by gpsd
diff --git a/test_libgps.c b/test_libgps.c
index 933beba0..d0837188 100644
--- a/test_libgps.c
+++ b/test_libgps.c
@@ -1,6 +1,7 @@
 /*
  * A simple command-line exerciser for the library.
  * Not really useful for anything but debugging.
+ * SPDX-License-Identifier: BSD-2-clause
  */
 #include <stdio.h>
 #include <stdlib.h>
@@ -111,7 +112,7 @@ int main(int argc, char *argv[])
 			"test_libgps: gps send error: %d, %s\n",
 			errno, gps_errstr(errno));
 	}
-	if (gps_read(&collect) == -1) {
+	if (gps_read(&collect, NULL, 0) == -1) {
 	  (void)fprintf(stderr,
 			"test_libgps: gps read error: %d, %s\n",
 			errno, gps_errstr(errno));
@@ -137,7 +138,7 @@ int main(int argc, char *argv[])
 	    }
 	    collect.set = 0;
 	    (void)gps_send(&collect, buf);
-	    (void)gps_read(&collect);
+	    (void)gps_read(&collect, NULL, 0);
 #ifdef SOCKET_EXPORT_ENABLE
 #ifdef LIBGPS_DEBUG
 	    libgps_dump_state(&collect);