I reviewed gpsd 3.20-1 as checked into focal. This shouldn't be considered a full audit but rather a quick gauge of maintainability. As noted in the MIR description, this review only covers code and binaries which are contained in the gpsd and libgps25 packages. gpsd is a daemon (and associated library) to handle input from GPS devices. The daemon translates the various GPS reciever inputs into a standard format and multiplexes this out to the various connected clients. It can also be used to connect to a remote gpsd instance as a source as well. For the purpose of this MIR, gpsd is the standard way to provide GPS time sources to chrony, which is now used as the time synchronisation daemon in Ubuntu. - CVE History: - 3 CVEs in project history (CVE-2004-1388 CVE-2013-2038 CVE-2018-17937) - 1 is still open for xenial, bionic and disco (CVE-2018-17937) - All fixed upstream in a timely manner - Build-Depends - Nothing of note here (no crypto / networking libraries) - pre/post inst/rm scripts - gpsd.postinst setups up the gpsd user account - gpsd.postrm removes the gpsd user account on purge - No init scripts - systemd units are used to spawn gpsdctl (see below) for GPS devices, and to manage the local GPSD socket and hence launch the GPSD daemon on demand when a local client connects - No dbus services - No setuid binaries - gpsd binary package provides the following binaries in PATH - /usr/bin/ppscheck - /usr/sbin/gpsd - /usr/sbin/gpsdctl - AppArmor policy: - /etc/apparmor.d/usr.sbin.gpsd - This looks quite sensible and provides access to the commonly used components and paths plus includes a site-specific profile to allow for easy customisation - No sudo fragments - Uses udev rules to ensure GPS devices have correct name (/dev/gps%n) and the right systemd service (gpsdctl@%k) is spawned - the gpsdctl service spawns /usr/sbin/gpsdctl for each device which inturn spawns a gpsd instance to read from that device and set the device permissions so that the gpsd binary can still read from the device once it drops privileges - unit tests / autopkgtests - Some unit tests are run during the build - autopkgtest exists - this does a basic test that the gpsd service and socket are available and active and that the output from the socket is the bare-minimum expected - No cron jobs - Build logs: - A single compile-time warning comes from an included system Qt header file using a deprecated copy method - /usr/include/x86_64-linux-gnu/qt5/QtCore/qvariant.h:275:25: warning: implicitly-declared 'constexpr QVariant::Private& QVariant::Private::operator=(const QVariant::Private&)' is deprecated [-Wdeprecated-copy] - Warning when generating man pages: - W: libgps-dev: manpage-has-errors-from-man usr/share/man/man5/gpsd_json.5.gz 272: warning [p 2, 2.2i, div '3tbd15,3', 0.8i]: can't break line - dpkg warnings: - dpkg-gensymbols: warning: debian/libqgpsmm25/DEBIAN/symbols doesn't match completely debian/libqgpsmm25.symbols - dpkg-gencontrol: warning: Recommends field of package gpsd: substitution variable ${python:Depends} used, but is not defined (this is also repeated for other packages but listed gpsd only here since this MIR is only for gpsd ) - Various lintian errors: - E: gpsd changes: bad-distribution-in-changes-file unstable - W: gpsd source: package-needs-versioned-debhelper-build-depends 12 - W: gpsd source: orig-tarball-missing-upstream-signature gpsd_3.20.orig.tar.xz - W: gpsd source: ancient-python-version-field x-python3-version 3.5 - E: gpsd-dbg: python-script-but-no-python-dep usr/lib/gpsd/debug/test_clienthelpers.py #!python - E: gpsd-dbg: python-script-but-no-python-dep usr/lib/gpsd/debug/test_misc.py #!python - W: libgps-dev: manpage-has-errors-from-man usr/share/man/man5/gpsd_json.5.gz 272: warning [p 2, 2.2i, div '3tbd15,3', 0.8i]: can't break line - W: gpsd-clients: executable-not-elf-or-script usr/share/doc/gpsd-clients/examples/skyview.php - N: 1 tag overridden (1 info) - E: Lintian run failed (policy violation) - Processes spawned - gpsd can be configured to run a hook script (/etc/gpsd/device-hook) when devices are activated / deactived - this is done by spawning system() with the path to the gps device and a fixed argument indicating if it was activated or deactivated. If an attacker were able to control the device name they would be able to get gpsd to execute arbitrary commands when the hook script is executed so whilst this is unlikely, this would be better if it was a call to execve() or similar to avoid possible shell-command injection in the call to system(). However, since it is unlikely an attacker could configure a device name which contained shell directives this is unlikely to be a practical threat. - Memory management - gpsd appears to demonstrate quite good memory management and no obvious instances of errors were found - memory allocations are checked for failure, memcpy()'s which might overflow bounds are checked etc. - File IO - PPS input is taken from a file in sysfs which is assumed trusted as is provided by the kernel - No files are created by gpsd - Logging - Logging is generally done via the GPSD_LOG() macro which expands to call the gpsd_log() function to allow runtime log-level control - this inturn eventually uses vsnprintf(). There does not appear to be any instances where format string vulnerabilities could be abused. There are also uses of syslog() but again this looks quite safe. - Environment variable usage - Uses GPSD_SHM_KEY when opening a connection to a GPSD daemon via shared memory - Uses LISTEN_PID and LISTEN_FDS to support systemd socket activation - These look safe - Use of privileged functions - Drops privileges to the dialout group and gpsd user correctly during execution - Changes device permissions of default devices (ones specified on the command-line) before dropping permissions to ensure can be read afterwards - No use of cryptography / random number sources etc - No use of temp files - Use of networking appears safe and sanitised appropriately - No use of WebKit - No use of PolicyKit - One cppcheck result: - [monitor_italk.c:166]: (error) Shifting signed 32-bit value by 31 bits is undefined behaviour - This was apparently fixed in upstream commit https://gitlab.com/gpsd/gpsd/commit/b1b555e0cb37382655a3a7cb37f2f89cb89eda7a however on some platforms (32-bit ARM and x86 in particular) the use of unsigned long is not enough (as it is still only 32-bits wide) so this should likely be a uint64_t to ensure there is no possible undefined behaviour by shifting beyond the size of the underlying type - Significant Coverity results: - Some sections of code use large stack allocated buffers (eg. 18436 bytes) which could possibly overrun the stack size but this is quite unlikely in practice as these are not deep code paths etc - See attached list of other coverity findings. gpsd appears to be quite well written and defensive. It contains a lot of bespoke code to handle various custom GPS receiver protocols but in general these appear to be safe. The daemon drops privileges safely and in general runs unprivileged and includes an AppArmor profile which provides quite good isolation from the rest of the system. Security team ACK for promoting the gpsd and libgps25 binary packages to main.