p2p-server: Only read 32 bytes from /var/lib/dbus/machine-id
At startup, p2p-server emits this error via syslog
2013-07-02T12:06:18.036161-07:00 localhost p2p-server: Error reading from /var/lib/dbus/machine-id, num_read=33 [service_publisher.cc:130]
This happens because the /var/lib/dbus/machine-id file can contain a
new-line so its file size ends up being 33 bytes and we read these 33
bytes into an array that is only 33 bytes long. This in turn means
there is no terminating NUL byte. Things more or less work anyway
since p2p-server ends up using the machine-id + newline as the service
name and the following byte usually is a NUL byte anyway.
The problem can easily be identified by running avahi-discover(1) on a
machine on the same LAN and inspecting that its output, for example
Found service '52b9e689edb6049ac025162a51d324a3
' of type '_cros_p2p._tcp' in domain 'local' on 3.0.
contains the newline in the service name.
This problem is fixed by simply reading at most 32 bytes from the
machine-id file and ensuring that the buffer we read into is NUL
terminated.
BUG=None
TEST=Ran avahi-discover(1) and check there's no newline in the service name
Change-Id: If96fda59207cd2584a4bb69eb6cafbf4ba6a3516
Reviewed-on: https://gerrit.chromium.org/gerrit/60799
Reviewed-by: Alex Deymo <deymo@chromium.org>
Tested-by: David Zeuthen <zeuthen@chromium.org>
Commit-Queue: David Zeuthen <zeuthen@chromium.org>
diff --git a/server/service_publisher.cc b/server/service_publisher.cc
index eea28f2..2655da5 100644
--- a/server/service_publisher.cc
+++ b/server/service_publisher.cc
@@ -31,6 +31,10 @@
// may end up generate a lot of unnecessary traffic.
const int kFileChangedDelayMSec = 10000;
+// The encoding of the D-Bus machine-id plus a NUL terminator is 33
+// bytes. See http://dbus.freedesktop.org/doc/dbus-specification.html
+const size_t kDBusMachineIdPlusNulSize = 33;
+
class ServicePublisherAvahi : public ServicePublisher {
public:
explicit ServicePublisherAvahi(uint16_t http_port);
@@ -113,20 +117,25 @@
avahi_glib_poll_free(poll_);
}
-// Reads the machine_id into |buf|.
-static void
-ReadMachineId(char *buf, size_t buf_size) {
+// Reads the D-Bus machine-id into |buf| and ensures that it's
+// NUL-terminated. It is a programming error to pass a |buf|
+// that is not at least |kDBusMachineIdPlusNulSize| bytes long.
+static void ReadMachineId(char *buf) {
size_t num_read = 0;
FILE* f = NULL;
+ // NUL-terminate ahead of time.
+ buf[kDBusMachineIdPlusNulSize - 1] = '\0';
+
f = fopen("/var/lib/dbus/machine-id", "r");
if (f == NULL) {
LOG(ERROR) << "Error opening /var/lib/dbus/machine-id: " << strerror(errno);
return;
}
- num_read = fread(buf, 1, buf_size, f);
- if (num_read != 32) {
+ // The machine-id file may include a newline so only request 32 bytes.
+ num_read = fread(buf, 1, kDBusMachineIdPlusNulSize - 1, f);
+ if (num_read != kDBusMachineIdPlusNulSize - 1) {
LOG(ERROR) << "Error reading from /var/lib/dbus/machine-id, num_read="
<< num_read;
fclose(f);
@@ -143,10 +152,12 @@
// This is not thread safe and blocks the calling thread the first
// time it is called.
static const char* GetDBusMachineId(void) {
- static char machine_id[33] = { 0 };
+ static char machine_id[kDBusMachineIdPlusNulSize] = { 0 };
- if (machine_id[0] == '\0')
- ReadMachineId(machine_id, sizeof machine_id);
+ if (machine_id[0] == '\0') {
+ G_STATIC_ASSERT(sizeof machine_id == kDBusMachineIdPlusNulSize);
+ ReadMachineId(machine_id);
+ }
return const_cast<const char *>(machine_id);
}
