Restrict the length of key and values when setting metadata

By setting very long keys and/or values over-and-over you can get the colord
daemon to crash with an out-of-memory error.
To prevent this, limit the length of the metadata key to 128 bytes and the
metadata value to 4k. Anything longer than that isn't really metadata...

Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1012832

5 files changed, 73 insertions, 12 deletions

diff --git a/src/cd-common.h b/src/cd-common.h
index 108cbe4..adf9343 100644
--- a/src/cd-common.h
+++ b/src/cd-common.h
@@ -34,6 +34,10 @@
 #define COLORD_DBUS_INTERFACE_PROFILE	"org.freedesktop.ColorManager.Profile"
 #define COLORD_DBUS_INTERFACE_SENSOR	"org.freedesktop.ColorManager.Sensor"
 
+#define CD_DBUS_METADATA_KEY_LEN_MAX	256	/* chars */
+#define CD_DBUS_METADATA_VALUE_LEN_MAX	4096	/* chars */
+
+
 #define CD_CLIENT_ERROR			cd_client_error_quark()
 
 GQuark		 cd_client_error_quark		(void);
diff --git a/src/cd-device.c b/src/cd-device.c
index 7ed90a0..2f7d901 100644
--- a/src/cd-device.c
+++ b/src/cd-device.c
@@ -930,6 +930,24 @@ cd_device_set_property_internal (CdDevice *device,
 	gboolean is_metadata = FALSE;
 	CdDevicePrivate *priv = device->priv;
 
+	/* sanity check the length of the key and value */
+	if (strlen (property) > CD_DBUS_METADATA_KEY_LEN_MAX) {
+		ret = FALSE;
+		g_set_error_literal (error,
+				     CD_CLIENT_ERROR,
+				     CD_CLIENT_ERROR_INPUT_INVALID,
+				     "metadata key length invalid");
+		goto out;
+	}
+	if (value != NULL && strlen (value) > CD_DBUS_METADATA_VALUE_LEN_MAX) {
+		ret = FALSE;
+		g_set_error_literal (error,
+				     CD_CLIENT_ERROR,
+				     CD_CLIENT_ERROR_INPUT_INVALID,
+				     "metadata value length invalid");
+		goto out;
+	}
+
 	g_debug ("CdDevice: Attempting to set %s to %s on %s",
 		 property, value, device->priv->id);
 	if (g_strcmp0 (property, CD_DEVICE_PROPERTY_MODEL) == 0) {
@@ -980,6 +998,7 @@ cd_device_set_property_internal (CdDevice *device,
 						      property,
 						      cd_device_get_nullable_for_string (value));
 	}
+out:
 	return ret;
 }
 
diff --git a/src/cd-main.c b/src/cd-main.c
index be54bac..d1185bb 100644
--- a/src/cd-main.c
+++ b/src/cd-main.c
@@ -1606,9 +1606,15 @@ cd_main_daemon_method_call (GDBusConnection *connection, const gchar *sender,
 					    &prop_key, &prop_value)) {
 			if (g_strcmp0 (prop_key, CD_PROFILE_PROPERTY_FILENAME) == 0)
 				filename = g_strdup (prop_value);
-			cd_profile_set_property_internal (profile,
-							  prop_key,
-							  prop_value);
+			ret = cd_profile_set_property_internal (profile,
+								prop_key,
+								prop_value,
+								&error);
+			if (!ret) {
+				g_dbus_method_invocation_return_gerror (invocation,
+									error);
+				goto out;
+			}
 		}
 
 		/* get any file descriptor in the message */
@@ -1787,7 +1793,8 @@ cd_main_icc_store_added_cb (CdIccStore *icc_store,
 	checksum = cd_profile_get_checksum (profile);
 	cd_profile_set_property_internal (profile,
 					  CD_PROFILE_METADATA_FILE_CHECKSUM,
-					  checksum);
+					  checksum,
+					  NULL);
 
 	/* just add it to the bus with the title as the ID */
 	profile_id = g_strdup_printf ("icc-%s", cd_icc_get_checksum (icc));
diff --git a/src/cd-profile.c b/src/cd-profile.c
index 782c8c1..a97b060 100644
--- a/src/cd-profile.c
+++ b/src/cd-profile.c
@@ -532,13 +532,34 @@ cd_profile_get_nullable_for_string (const gchar *value)
 /**
  * cd_profile_set_property_internal:
  **/
-void
+gboolean
 cd_profile_set_property_internal (CdProfile *profile,
 				  const gchar *property,
-				  const gchar *value)
+				  const gchar *value,
+				  GError **error)
 {
+	gboolean ret = TRUE;
+
 	CdProfilePrivate *priv = profile->priv;
 
+	/* sanity check the length of the key and value */
+	if (strlen (property) > CD_DBUS_METADATA_KEY_LEN_MAX) {
+		ret = FALSE;
+		g_set_error_literal (error,
+				     CD_CLIENT_ERROR,
+				     CD_CLIENT_ERROR_INPUT_INVALID,
+				     "metadata key length invalid");
+		goto out;
+	}
+	if (value != NULL && strlen (value) > CD_DBUS_METADATA_VALUE_LEN_MAX) {
+		ret = FALSE;
+		g_set_error_literal (error,
+				     CD_CLIENT_ERROR,
+				     CD_CLIENT_ERROR_INPUT_INVALID,
+				     "metadata value length invalid");
+		goto out;
+	}
+
 	if (g_strcmp0 (property, CD_PROFILE_PROPERTY_FILENAME) == 0) {
 		cd_profile_set_filename (profile, value);
 		cd_profile_dbus_emit_property_changed (profile,
@@ -565,11 +586,13 @@ cd_profile_set_property_internal (CdProfile *profile,
 		cd_profile_dbus_emit_property_changed (profile,
 						       CD_PROFILE_PROPERTY_METADATA,
 						       cd_profile_get_metadata_as_variant (profile));
-		return;
+		goto out;
 	}
 
 	/* emit global signal */
 	cd_profile_dbus_emit_profile_changed (profile);
+out:
+	return ret;
 }
 
 /**
@@ -619,9 +642,16 @@ cd_profile_dbus_method_call (GDBusConnection *connection, const gchar *sender,
 							       property_name);
 			goto out;
 		}
-		cd_profile_set_property_internal (profile,
-						  property_name,
-						  property_value);
+		ret = cd_profile_set_property_internal (profile,
+							property_name,
+							property_value,
+							&error);
+		if (!ret) {
+			g_dbus_method_invocation_return_gerror (invocation,
+								error);
+			g_error_free (error);
+			goto out;
+		}
 		g_dbus_method_invocation_return_value (invocation, NULL);
 		goto out;
 	}
diff --git a/src/cd-profile.h b/src/cd-profile.h
index f10c5b0..d893a8a 100644
--- a/src/cd-profile.h
+++ b/src/cd-profile.h
@@ -104,9 +104,10 @@ CdColorspace	 cd_profile_get_colorspace		(CdProfile	*profile);
 gboolean	 cd_profile_get_has_vcgt		(CdProfile	*profile);
 void		 cd_profile_watch_sender		(CdProfile	*profile,
 							 const gchar	*sender);
-void		 cd_profile_set_property_internal	(CdProfile	*profile,
+gboolean	 cd_profile_set_property_internal	(CdProfile	*profile,
 							 const gchar	*property,
-							 const gchar	*value);
+							 const gchar	*value,
+							 GError		**error);
 const gchar	**cd_profile_get_warnings		(CdProfile	*profile);
 
 G_END_DECLS