Check incoming ids for validity

In libei we just check the range is correct.
In libeis we also check that the client is monotonically increasing the
id.

src/brei-shared.h

@@ -160,3 +160,13 @@ brei_dispatch(struct brei_context *brei,
  */
 void
 brei_drain_fd(int fd);
+
+static inline bool
+brei_is_server_id(object_id_t id) {
+	return id >= 0xff00000000000000;
+}
+
+static inline bool
+brei_is_client_id(object_id_t id) {
+	return id != 0 && id < 0xff00000000000000;
+}

src/libei-device.c

@@ -274,6 +274,14 @@ handle_msg_paused(struct ei_device *device, uint32_t serial)
 	return NULL;
 }
 
+#define DISCONNECT_IF_INVALID_ID(device_, id_) do { \
+	if (!brei_is_server_id(id_)) { \
+		struct ei *ei_ = ei_device_get_context(device_); \
+		log_bug(ei_, "Received invalid object id %#" PRIx64 ". Disconnecting", id_); \
+		return brei_result_new(EI_CONNECTION_DISCONNECT_REASON_PROTOCOL, "Received invalid object id %#" PRIx64 ".", id_); \
+	} \
+} while(0)
+
 #define DISCONNECT_IF_SENDER_CONTEXT(device_) do {\
 	struct ei *ei_ = ei_device_get_context(device_); \
 	if (ei_is_sender(ei_)) { \
@@ -382,6 +390,8 @@ handle_msg_frame(struct ei_device *device, uint32_t serial, uint64_t time)
 static struct brei_result *
 handle_msg_pointer(struct ei_device *device, object_id_t id, uint32_t version)
 {
+	DISCONNECT_IF_INVALID_ID(device, id);
+
 	if (device->pointer)
 		return brei_result_new(EI_CONNECTION_DISCONNECT_REASON_PROTOCOL,
 				       "Duplicate ei_pointer interface object on device");
@@ -394,6 +404,8 @@ handle_msg_pointer(struct ei_device *device, object_id_t id, uint32_t version)
 static struct brei_result *
 handle_msg_keyboard(struct ei_device *device, object_id_t id, uint32_t version)
 {
+	DISCONNECT_IF_INVALID_ID(device, id);
+
 	if (device->keyboard)
 		return brei_result_new(EI_CONNECTION_DISCONNECT_REASON_PROTOCOL,
 				       "Duplicate ei_keyboard interface object on device");
@@ -406,6 +418,8 @@ handle_msg_keyboard(struct ei_device *device, object_id_t id, uint32_t version)
 static struct brei_result *
 handle_msg_touchscreen(struct ei_device *device, object_id_t id, uint32_t version)
 {
+	DISCONNECT_IF_INVALID_ID(device, id);
+
 	if (device->touchscreen)
 		return brei_result_new(EI_CONNECTION_DISCONNECT_REASON_PROTOCOL,
 				       "Duplicate ei_touchscreen interface object on device");

src/libei-seat.c

@@ -118,9 +118,19 @@ handle_msg_done(struct ei_seat *seat)
 	return NULL;
 }
 
+#define DISCONNECT_IF_INVALID_ID(seat_, id_) do { \
+	if (!brei_is_server_id(id_)) { \
+		struct ei *ei_ = ei_seat_get_context(seat_); \
+		log_bug(ei_, "Received invalid object id %#" PRIx64 ". Disconnecting", id_); \
+		return brei_result_new(EI_CONNECTION_DISCONNECT_REASON_PROTOCOL, "Received invalid object id %#" PRIx64 ".", id_); \
+	} \
+} while(0)
+
 static struct brei_result *
 handle_msg_device(struct ei_seat *seat, object_id_t id, uint32_t version)
 {
+	DISCONNECT_IF_INVALID_ID(seat, id);
+
 	struct ei *ei = ei_seat_get_context(seat);
 
 	log_debug(ei, "Added device %#" PRIx64 "@v%u", id, version);

src/libei.c

@@ -596,9 +596,19 @@ ei_disconnect(struct ei *ei)
 	ei->source = source_unref(ei->source);
 }
 
+#define DISCONNECT_IF_INVALID_ID(connection_, id_) do { \
+	if (!brei_is_server_id(id_)) { \
+		struct ei *ei_ = ei_connection_get_context(connection_); \
+		log_bug(ei_, "Received invalid object id %#" PRIx64 ". Disconnecting", id_); \
+		return brei_result_new(EI_CONNECTION_DISCONNECT_REASON_PROTOCOL, "Received invalid object id %#" PRIx64 ".", id_); \
+	} \
+} while(0)
+
 static struct brei_result *
 handle_msg_seat(struct ei_connection *connection, object_id_t seat_id, uint32_t version)
 {
+	DISCONNECT_IF_INVALID_ID(connection, seat_id);
+
 	struct ei *ei = ei_connection_get_context(connection);
 	struct ei_seat *seat = ei_seat_new(ei, seat_id, version);
 
@@ -692,6 +702,8 @@ handle_msg_invalid_object(struct ei_connection *connection, uint32_t last_serial
 static struct brei_result *
 handle_msg_ping(struct ei_connection *connection, object_id_t id,  uint32_t version)
 {
+	DISCONNECT_IF_INVALID_ID(connection, id);
+
 	struct ei *ei = ei_connection_get_context(connection);
 	_unref_(ei_pingpong) *pingpong = ei_pingpong_new_for_id(ei, id, version);
 

src/libeis-client.c

@@ -109,6 +109,16 @@ eis_client_get_new_id(struct eis_client *client)
 	return offset | (client->next_object_id++ & mask);
 }
 
+bool
+eis_client_update_client_object_id(struct eis_client *client, object_id_t id)
+{
+	if (id <= client->last_client_object_id)
+		return false;
+
+	client->last_client_object_id = id;
+	return true;
+}
+
 void
 eis_client_register_object(struct eis_client *client, struct brei_object *object)
 {
@@ -279,6 +289,14 @@ eis_client_setup_done(struct eis_client *client, const char *name, bool is_sende
 	 */
 }
 
+#define DISCONNECT_IF_INVALID_ID(client_, id_) do { \
+	if (!brei_is_client_id(id_) || !eis_client_update_client_object_id(client, id_)) { \
+		struct eis *_ctx = eis_client_get_context(client_); \
+		log_bug_client(_ctx, "Invalid object id %#" PRIx64 ". Disconnecting client", id_); \
+		return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_PROTOCOL, "Invalid object id %#" PRIx64 ".", new_id); \
+	} \
+} while(0)
+
 static struct brei_result *
 client_msg_disconnect(struct eis_connection *connection)
 {
@@ -290,12 +308,10 @@ client_msg_disconnect(struct eis_connection *connection)
 static struct brei_result *
 client_msg_sync(struct eis_connection *connection, object_id_t new_id)
 {
-	if (new_id == 0) {
-		return brei_result_new(EIS_CONNECTION_DISCONNECT_REASON_VALUE,
-				       "Invalid object id zero for ei_connection.sync");
-	}
-
 	struct eis_client *client = eis_connection_get_client(connection);
+
+	DISCONNECT_IF_INVALID_ID(client, new_id);
+
 	struct eis_callback *callback = eis_callback_new(client, new_id, client->interface_versions.ei_callback);
 	log_debug(eis_client_get_context(client) , "object %#" PRIx64 ": connection sync done", new_id);
 	int rc = eis_callback_event_done(callback, 0);

src/libeis-client.h

@@ -57,6 +57,7 @@ struct eis_client {
 
 	struct list proto_objects; /* struct brei_objects list */
 	object_id_t next_object_id;
+	object_id_t last_client_object_id;
 
 	uint32_t serial;
 	uint32_t last_client_serial;
@@ -99,6 +100,9 @@ eis_client_update_client_serial(struct eis_client *client, uint32_t serial);
 object_id_t
 eis_client_get_new_id(struct eis_client *client);
 
+bool
+eis_client_update_client_object_id(struct eis_client *client, object_id_t id);
+
 void
 eis_client_register_object(struct eis_client *client, struct brei_object *object);
 

test/test_protocol.py

@@ -828,3 +828,60 @@ class TestEiProtocol:
 
         except BrokenPipeError:
             assert missing_interface in ["ei_connection", "ei_callback", "ei_pingpong"]
+
+    @pytest.mark.parametrize("test_for", ("repeat-id", "invalid-id", "decreasing-id"))
+    def test_invalid_object_id(self, eis, test_for):
+        """
+        Expect to get disconnected if we allocate a client ID in the server ID range
+        or if we allocate the same client id twice
+        """
+        ei = eis.ei
+        ei.dispatch()
+
+        ei.init_default_sender_connection()
+        ei.dispatch()
+        ei.wait_for_connection()
+
+        @attr.s
+        class Status:
+            disconnected: bool = attr.ib(default=False)
+            reason: int = attr.ib(default=0)
+            explanation: Optional[str] = attr.ib(default=None)
+
+        status = Status()
+
+        def on_disconnected(connection, last_serial, reason, explanation):
+            status.disconnected = True
+            status.reason = reason
+            status.explanation = explanation
+
+        ei.connection.connect("Disconnected", on_disconnected)
+
+        if test_for == "invalid-id":
+            # random id in the server range, 0xff..00 is used by the connection
+            # and some of the next few ids might have been used by pingpongs
+            cb = EiCallback.create(0xFF00000000000100, VERSION_V(1))
+            ei.context.register(cb)
+            ei.send(ei.connection.Sync(cb.object_id))
+        elif test_for == "repeat-id":
+            cb = EiCallback.create(0x100, VERSION_V(1))
+            ei.context.register(cb)
+            ei.send(ei.connection.Sync(cb.object_id))
+            cb = EiCallback.create(0x100, VERSION_V(1))
+            ei.send(ei.connection.Sync(cb.object_id))
+        elif test_for == "decreasing-id":
+            cb = EiCallback.create(0x101, VERSION_V(1))
+            ei.context.register(cb)
+            ei.send(ei.connection.Sync(cb.object_id))
+            cb = EiCallback.create(0x100, VERSION_V(1))
+            ei.context.register(cb)
+            ei.send(ei.connection.Sync(cb.object_id))
+        else:
+            assert False, "Unhandled test parameter"
+
+        ei.wait_for(lambda: status.disconnected)
+        assert status.disconnected
+        assert (
+            status.reason == EiConnection.EiDisconnectReason.PROTOCOL
+        ), status.explanation
+        assert status.explanation is not None