Bug 1212951

OBS devel project: network:time/nqptp

Overview
========

nqptp [0] is a Precision Time Protocol [1] daemon that implements part
of PTP, acting only as a client, not as a clock source. It is developed
as a companion to `shairport-sync` [2], an audio player for Apple
AirPlay 2.

It listens to incoming PTP messages on ports 319/udp and 320/udp and
subsequently provides clock data to other applications via a POSIX
shared memory object `/dev/shm/nqptp`.  There's also a local control
interface on 9000/udp.


Privileges
==========

nqptp is currently designed to run as root. It turns out the only
privileged operation required is binding to port < 1024.

Suggestion:
-----------

Don't run as root and instead use a more focused filesystem
capability [3] to allow it to bind the necessary ports, e.g.:
`setcap cap_net_bind_service+ep nqptp`


Shared Memory Object
====================

nqptp is designed to interoperate with other programs via a POSIX shared
memory object in `/dev/shm/nqptp`. This object contains a
`struct shm_structure` with information on the current clock status:

```
struct shm_structure {
  pthread_mutex_t shm_mutex;            // for safely accessing the structure
  uint16_t version;                     // check this is equal to NQPTP_SHM_STRUCTURES_VERSION
  uint64_t master_clock_id;             // the current master clock
  char master_clock_ip[64];             // where it's coming from
  uint64_t local_time;                  // the time when the offset was calculated
  uint64_t local_to_master_time_offset; // add this to the local time to get master clock time
  uint64_t master_clock_start_time;     // this is when the master clock became master
};
```

All fields, except the mutex, are only written to by nqptp, not read.
The SHM is created with world-writable privileges, allowing any local
user to modify it freely:

main.c:200:
```
shm_fd = shm_open(NQPTP_INTERFACE_NAME, O_RDWR | O_CREAT, 0666);
```

This allows unprivileged users to corrupt the integrity of any programs
that rely on the contents of this object, and possibly wreak havoc by
manipulating the mutex, though I have not dug deeper into that.

Suggestion:
-----------

Restrict /dev/shm/nqptp to 0660, and an appropriate user/group instead
of root.


Protocol handlers
=================

The handler for the control protocol (port 9000/udp) can be crashed by
sending a packet with a payload that does not contain a space character,
e.g. an empty packet:

> nc -w0 -u 127.0.0.1 9000 <<< ""

> AddressSanitizer:DEADLYSIGNAL
> =================================================================
> ==7787==ERROR: AddressSanitizer: SEGV on unknown address
> 0x000000000000 (pc 0x7f19d7ac57cd bp 0x7ffe74976fe0 sp 0x7ffe74976770
> T0)
> ==7787==The signal is caused by a READ memory access.

Suggestion:
-----------

Harden the protocol handlers against unexpected input.


References
==========

[0] https://github.com/mikebrady/nqptp
[1] https://en.wikipedia.org/wiki/Precision_Time_Protocol
[2] https://github.com/mikebrady/shairport-sync
[3] capabilities(7)

AUDIT-FIND bugs:

https://bugzilla.suse.com/show_bug.cgi?id=1213060
https://bugzilla.suse.com/show_bug.cgi?id=1213526

I reported this to upstream in https://github.com/mikebrady/nqptp/security/advisories/GHSA-wmw7-5qrp-9w4g

All findings (and more) have been addressed by upstream:
https://github.com/mikebrady/nqptp/releases/tag/1.2.4

Done.