Make the socket connection non-blocking and limit conversations to 20

Thu Oct 16 09:13:48 2003  George Lebl <jirka@5z.com>

	* daemon/gdm.[ch], daemon/gdm-net.[ch]: Make the socket connection
	  non-blocking and limit conversations to 20 requests.  Also
	  cut lines short at 4096.  Fixes CAN-2003-0793 and CAN-2003-0794
	  respectively.

	* daemon/slave.c: fix #123958 by clearing the message after
	  authentication is done for the configurator.

	* daemon/Makefile.am, gui/greeter/Makefile.am, gui/Makefile.am,
	  gui/modules/Makefile.am:  The disable_deprecated defines should
	  only be on for conformance testing I suppose.  Fixes #124680

11 files changed, 159 insertions, 24 deletions

diff --git a/ChangeLog b/ChangeLog
index af1b3d9c..501efd19 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,17 @@
+Thu Oct 16 09:13:48 2003  George Lebl <jirka@5z.com>
+
+	* daemon/gdm.[ch], daemon/gdm-net.[ch]: Make the socket connection
+	  non-blocking and limit conversations to 20 requests.  Also
+	  cut lines short at 4096.  Fixes CAN-2003-0793 and CAN-2003-0794
+	  respectively.
+
+	* daemon/slave.c: fix #123958 by clearing the message after
+	  authentication is done for the configurator.
+
+	* daemon/Makefile.am, gui/greeter/Makefile.am, gui/Makefile.am,
+	  gui/modules/Makefile.am:  The disable_deprecated defines should
+	  only be on for conformance testing I suppose.  Fixes #124680
+
 Wed Oct 15 14:19:59 2003  George Lebl <jirka@5z.com>
 
 	* gui/greeter/greeter_item.c: the rich string parsing routine was
diff --git a/NEWS b/NEWS
index 24ad6ab9..0630bcc8 100644
--- a/NEWS
+++ b/NEWS
@@ -2,6 +2,16 @@ Ahh news...
 
 2.4.4.4 stuff:
 
+- SECURITY: Fixed CAN-2003-0793, a local DoS, the socket connection
+  is now non-blocking and limitted to the number of commands
+
+- SECURITY: Fixed CAN-2003-0794, a local DoS, the line length is limitted
+  to 4096 bytes (note, this was not a buffer overrun).
+
+  (Thanks to Jarno Gassenbauer for pointing out the above two problems)
+
+- Avoid possible DoS by using "-audit 0" for the X server command line
+
 - When cookies are in the fallback dir touch them every
   12 hours to avoid tmpwatch from removing them
 
@@ -17,7 +27,13 @@ Ahh news...
 
 - Some more anality with touching user owned files
 
-- Minor other fixes
+- Fixed the graphical greeter line breaking to not upset
+  pango and generally work with marked up strings
+
+- Fix an underlining bug in the graphical greeter when the underlined letter
+  is the last letter. (discussed in rh #106189)
+
+- Minor other fixes (among others #123958, #124680)
 
 2.4.4.3 stuff:
 
diff --git a/daemon/Makefile.am b/daemon/Makefile.am
index 28ecc235..9f8742d5 100644
--- a/daemon/Makefile.am
+++ b/daemon/Makefile.am
@@ -6,11 +6,6 @@ DEFS = @DEFS@ -DGDM_CONFIG_FILE=\"@sysconfdir@/gdm/gdm.conf\"
 INCLUDES = \
 	-I.						\
 	-I..						\
-	-DG_DISABLE_DEPRECATED				\
-	-DGDK_DISABLE_DEPRECATED			\
-	-DGDK_PIXBUF_DISABLE_DEPRECATED			\
-	-DGTK_DISABLE_DEPRECATED			\
-	-DGNOME_DISABLE_DEPRECATED			\
 	-I$(top_srcdir)/vicious-extensions		\
 	-DGNOMELOCALEDIR=\""$(datadir)/locale"\"	\
 	-DLOCALEDIR=\"$(datadir)/locale\"		\
@@ -19,6 +14,13 @@ INCLUDES = \
 	-DPAM_PREFIX=\"$(PAM_PREFIX)\"			\
 	$(GNOME_INCLUDEDIR)
 
+#
+#	-DG_DISABLE_DEPRECATED				\
+#	-DGDK_DISABLE_DEPRECATED			\
+#	-DGDK_PIXBUF_DISABLE_DEPRECATED			\
+#	-DGTK_DISABLE_DEPRECATED			\
+#	-DGNOME_DISABLE_DEPRECATED			\
+#
 
 bin_PROGRAMS = gdm-binary
 
diff --git a/daemon/gdm-net.c b/daemon/gdm-net.c
index 6e91dab5..a0161241 100644
--- a/daemon/gdm-net.c
+++ b/daemon/gdm-net.c
@@ -48,6 +48,10 @@ struct _GdmConnection {
 
 	GString *buffer;
 
+	int message_count;
+
+	gboolean nonblock;
+
 	int close_level; /* 0 - normal
 			    1 - no close, when called raise to 2
 			    2 - close was requested */
@@ -115,8 +119,11 @@ gdm_connection_handler (GIOChannel *source,
 		     ve_string_empty (conn->buffer->str)))
 			/*ignore \r or empty lines*/
 			continue;
-		if (*p == '\n') {
+		if (*p == '\n' || 
+		    /* cut lines short at 4096 to prevent DoS attacks */
+		    conn->buffer->len > 4096) {
 			conn->close_level = 1;
+			conn->message_count ++;
 			conn->handler (conn, conn->buffer->str,
 				       conn->data);
 			if (conn->close_level == 2) {
@@ -147,6 +154,8 @@ gboolean
 gdm_connection_write (GdmConnection *conn, const char *str)
 {
 	int ret;
+	int save_errno;
+	int flags = 0;
 #ifndef MSG_NOSIGNAL
 	void (*old_handler)(int);
 #endif
@@ -157,14 +166,24 @@ gdm_connection_write (GdmConnection *conn, const char *str)
 	if G_UNLIKELY ( ! conn->writable)
 		return FALSE;
 
+#ifdef MSG_DONTWAIT
+	if (conn->nonblock)
+		flags |= MSG_DONTWAIT;
+#endif
+
 #ifdef MSG_NOSIGNAL
-	IGNORE_EINTR (ret = send (conn->fd, str, strlen (str), MSG_NOSIGNAL));
+	IGNORE_EINTR (ret = send (conn->fd, str, strlen (str), MSG_NOSIGNAL | flags));
+	save_errno = errno;
 #else
 	old_handler = signal (SIGPIPE, SIG_IGN);
-	IGNORE_EINTR (ret = send (conn->fd, str, strlen (str), 0));
+	IGNORE_EINTR (ret = send (conn->fd, str, strlen (str), flags));
+	save_errno = errno;
 	signal (SIGPIPE, old_handler);
 #endif
 
+	/* just so that 'signal' doesn't whack it */
+	errno = save_errno;
+
 	if G_UNLIKELY (ret < 0)
 		return FALSE;
 	else
@@ -197,6 +216,8 @@ gdm_socket_handler (GIOChannel *source,
 	gdm_debug ("gdm_socket_handler: Accepting new connection fd %d", fd);
 
 	newconn = g_new0 (GdmConnection, 1);
+	newconn->message_count = 0;
+	newconn->nonblock = conn->nonblock;
 	newconn->close_level = 0;
 	newconn->fd = fd;
 	newconn->writable = TRUE;
@@ -288,6 +309,8 @@ try_again:
 	IGNORE_EINTR (chmod (sockname, mode));
 
 	conn = g_new0 (GdmConnection, 1);
+	conn->message_count = 0;
+	conn->nonblock = FALSE;
 	conn->close_level = 0;
 	conn->fd = fd;
 	conn->writable = FALSE;
@@ -322,6 +345,8 @@ gdm_connection_open_fd (int fd)
 	g_return_val_if_fail (fd >= 0, NULL);
 
 	conn = g_new0 (GdmConnection, 1);
+	conn->message_count = 0;
+	conn->nonblock = FALSE;
 	conn->close_level = 0;
 	conn->fd = fd;
 	conn->writable = FALSE;
@@ -371,6 +396,8 @@ gdm_connection_open_fifo (const char *fifo, mode_t mode)
 	IGNORE_EINTR (chmod (fifo, mode));
 
 	conn = g_new0 (GdmConnection, 1);
+	conn->message_count = 0;
+	conn->nonblock = FALSE;
 	conn->close_level = 0;
 	conn->fd = fd;
 	conn->writable = FALSE;
@@ -519,4 +546,26 @@ gdm_connection_printf (GdmConnection *conn, const gchar *format, ...)
 	return ret;
 }
 
+int
+gdm_connection_get_message_count (GdmConnection *conn)
+{
+	g_return_val_if_fail (conn != NULL, -1);
+	return conn->message_count;
+}
+
+gboolean
+gdm_connection_get_nonblock (GdmConnection *conn)
+{
+	g_return_val_if_fail (conn != NULL, FALSE);
+	return conn->nonblock;
+}
+
+void
+gdm_connection_set_nonblock (GdmConnection *conn,
+			     gboolean nonblock)
+{
+	g_return_if_fail (conn != NULL);
+	conn->nonblock = nonblock;
+}
+
 /* EOF */
diff --git a/daemon/gdm-net.h b/daemon/gdm-net.h
index dda61c0b..8a94ea37 100644
--- a/daemon/gdm-net.h
+++ b/daemon/gdm-net.h
@@ -53,6 +53,10 @@ void		gdm_connection_set_handler (GdmConnection *conn,
 					    gpointer data,
 					    GDestroyNotify destroy_notify);
 
+gboolean	gdm_connection_get_nonblock   (GdmConnection *conn);
+void		gdm_connection_set_nonblock   (GdmConnection *conn,
+					       gboolean nonblock);
+
 guint32		gdm_connection_get_user_flags (GdmConnection *conn);
 void		gdm_connection_set_user_flags (GdmConnection *conn,
 					       guint32 flags);
@@ -67,6 +71,9 @@ void		gdm_connection_set_user_flags (GdmConnection *conn,
 			gdm_connection_set_user_flags (conn, _flags);		\
 		}
 
+int		gdm_connection_get_message_count (GdmConnection *conn);
+
+
 void		gdm_connection_close (GdmConnection *conn);
 
 #endif /* GDM_NET_H */
diff --git a/daemon/gdm.c b/daemon/gdm.c
index 49de5349..42c80398 100644
--- a/daemon/gdm.c
+++ b/daemon/gdm.c
@@ -1703,6 +1703,7 @@ create_connections (void)
 					    gdm_handle_user_message,
 					    NULL /* data */,
 					    NULL /* destroy_notify */);
+		gdm_connection_set_nonblock (unixconn, TRUE);
 		gdm_connection_set_close_notify (unixconn,
 						 &unixconn,
 						 close_notify);
@@ -3219,6 +3220,12 @@ gdm_handle_user_message (GdmConnection *conn, const char *msg, gpointer data)
 {
 	gdm_debug ("Handling user message: '%s'", msg);
 
+	if (gdm_connection_get_message_count (conn) > 20) {
+		gdm_connection_write (conn, "ERROR 200 Too many messages\n");
+		gdm_connection_close (conn);
+		return;
+	}
+
 	if (strncmp (msg, GDM_SUP_AUTH_LOCAL " ",
 		     strlen (GDM_SUP_AUTH_LOCAL " ")) == 0) {
 		GSList *li;
diff --git a/daemon/gdm.h b/daemon/gdm.h
index 9ca55790..b557906a 100644
--- a/daemon/gdm.h
+++ b/daemon/gdm.h
@@ -529,6 +529,9 @@ void		gdm_final_cleanup	(void);
  * is the gdm version and not a "protocol" revision, so you can't check
  * against a single version but check if the version is higher then some
  * value.
+ *
+ * You can only send a few commands at a time, so if you keep getting error
+ * 200 try opening a new socket for every command you send.
  */
 /* The user protocol, using /tmp/.gdm_socket */
 
@@ -538,6 +541,9 @@ void		gdm_final_cleanup	(void);
  * Arguments:  None
  * Answers:
  *   GDM <gdm version>
+ *   ERROR <err number> <english error description>
+ *      200 = Too many messages
+ *      999 = Unknown error
  */
 #define GDM_SUP_AUTH_LOCAL "AUTH_LOCAL" /* <xauth cookie> */
 /* AUTH_LOCAL: Setup this connection as authenticated for FLEXI_SERVER
@@ -554,6 +560,7 @@ void		gdm_final_cleanup	(void);
  *   ERROR <err number> <english error description>
  *      0 = Not implemented
  *      100 = Not authenticated
+ *      200 = Too many messages
  *      999 = Unknown error
  */
 #define GDM_SUP_FLEXI_XSERVER "FLEXI_XSERVER" /* <xserver type> */
@@ -572,6 +579,7 @@ void		gdm_final_cleanup	(void);
  *      4 = X too busy
  *      6 = No server binary
  *      100 = Not authenticated
+ *      200 = Too many messages
  *      999 = Unknown error
  */
 #define GDM_SUP_FLEXI_XNEST  "FLEXI_XNEST" /* <display> <uid> <xauth cookie> <xauth file> */
@@ -600,6 +608,7 @@ void		gdm_final_cleanup	(void);
  *      5 = Xnest can't connect
  *      6 = No server binary
  *      100 = Not authenticated
+ *      200 = Too many messages
  *      999 = Unknown error
  */
 #define GDM_SUP_CONSOLE_SERVERS  "CONSOLE_SERVERS" /* None */
@@ -617,6 +626,11 @@ void		gdm_final_cleanup	(void);
  *   for example).  If the display is an xnest display and is a console one
  *   (that is, it is an xnest inside another console display) it is listed
  *   and instead of vt, it lists the parent display in standard form.
+ *
+ *   ERROR <err number> <english error description>
+ *      1 = Not implemented
+ *      200 = Too many messages
+ *      999 = Unknown error
  */
 #define GDM_SUP_ALL_SERVERS  "ALL_SERVERS" /* None */
 /* ALL_SERVERS: List all servers, including console, remote, xnest.  This
@@ -631,6 +645,11 @@ void		gdm_final_cleanup	(void);
  *   <server> is <display>,<logged in user>
  *
  *   <logged in user> can be empty in case no one logged in yet
+ *
+ *   ERROR <err number> <english error description>
+ *      0 = Not implemented
+ *      200 = Too many messages
+ *      999 = Unknown error
  */
 #define GDM_SUP_UPDATE_CONFIG "UPDATE_CONFIG" /* <key> */
 /* UPDATE_CONFIG: Tell the daemon to update config of some key.  Any user
@@ -670,6 +689,7 @@ void		gdm_final_cleanup	(void);
  *   ERROR <err number> <english error description>
  *      0 = Not implemented
  *      50 = Unsupported key
+ *      200 = Too many messages
  *      999 = Unknown error
  */
 #define GDM_SUP_GREETERPIDS  "GREETERPIDS" /* None */
@@ -679,6 +699,10 @@ void		gdm_final_cleanup	(void);
  * Arguments:  None
  * Answers:
  *   OK <pid>;<pid>;...
+ *   ERROR <err number> <english error description>
+ *      0 = Not implemented
+ *      200 = Too many messages
+ *      999 = Unknown error
  */
 #define GDM_SUP_CLOSE        "CLOSE" /* no arguments */
 /* CLOSE Answers: None
diff --git a/daemon/slave.c b/daemon/slave.c
index 17be3e7f..7850f254 100644
--- a/daemon/slave.c
+++ b/daemon/slave.c
@@ -1743,6 +1743,10 @@ gdm_slave_wait_for_login (void)
 			login = NULL;
 			/* clear any error */
 			gdm_slave_greeter_ctl_no_ret (GDM_ERRBOX, "");
+			/* FIXME: what if the root has different 
+			   authentication?  This message ought to be changed
+			   to be more general, like "you must authenticate as root"
+			   or some such */
 			gdm_slave_greeter_ctl_no_ret
 				(GDM_MSG,
 				 _("Enter the root password\n"
@@ -1766,6 +1770,9 @@ gdm_slave_wait_for_login (void)
 						 d->console);
 			GdmAllowRoot = oldAllowRoot;
 
+			/* Clear message */
+			gdm_slave_greeter_ctl_no_ret (GDM_MSG, "");
+
 			if G_UNLIKELY (do_restart_greeter) {
 				g_free (login);
 				login = NULL;
diff --git a/gui/Makefile.am b/gui/Makefile.am
index 66618c51..aa050e03 100644
--- a/gui/Makefile.am
+++ b/gui/Makefile.am
@@ -13,16 +13,19 @@ DEFS = @DEFS@ \
 INCLUDES = \
 	-I.						\
 	-I..						\
-	-DG_DISABLE_DEPRECATED				\
-	-DGDK_DISABLE_DEPRECATED			\
-	-DGDK_PIXBUF_DISABLE_DEPRECATED			\
-	-DGTK_DISABLE_DEPRECATED			\
-	-DGNOME_DISABLE_DEPRECATED			\
 	-I$(top_srcdir)/daemon				\
 	-I$(top_srcdir)/vicious-extensions		\
 	-DGNOMELOCALEDIR=\""$(datadir)/locale"\" 	\
 	$(GUI_CFLAGS)
 
+#
+#	-DG_DISABLE_DEPRECATED				\
+#	-DGDK_DISABLE_DEPRECATED			\
+#	-DGDK_PIXBUF_DISABLE_DEPRECATED			\
+#	-DGTK_DISABLE_DEPRECATED			\
+#	-DGNOME_DISABLE_DEPRECATED			\
+#
+
 noinst_LIBRARIES = libgdmwm.a libgdmlang.a
 
 bin_PROGRAMS = \
diff --git a/gui/greeter/Makefile.am b/gui/greeter/Makefile.am
index cd151c27..0ff6b4a4 100644
--- a/gui/greeter/Makefile.am
+++ b/gui/greeter/Makefile.am
@@ -3,11 +3,6 @@ SUBDIRS = . themes
 ## Process this file with automake to produce makefile.in
 INCLUDES = \
 	-I.						\
-	-DG_DISABLE_DEPRECATED				\
-	-DGDK_DISABLE_DEPRECATED			\
-	-DGDK_PIXBUF_DISABLE_DEPRECATED			\
-	-DGTK_DISABLE_DEPRECATED			\
-	-DGNOME_DISABLE_DEPRECATED			\
 	-I$(top_srcdir)/gui				\
 	-I$(top_srcdir)/daemon				\
 	-I$(top_srcdir)/vicious-extensions		\
@@ -17,6 +12,14 @@ INCLUDES = \
 	$(GUI_CFLAGS)					\
 	$(GREETER_CFLAGS)
 
+#
+#	-DG_DISABLE_DEPRECATED				\
+#	-DGDK_DISABLE_DEPRECATED			\
+#	-DGDK_PIXBUF_DISABLE_DEPRECATED			\
+#	-DGTK_DISABLE_DEPRECATED			\
+#	-DGNOME_DISABLE_DEPRECATED			\
+#
+
 bin_PROGRAMS = \
 	gdmgreeter
 
diff --git a/gui/modules/Makefile.am b/gui/modules/Makefile.am
index 14012672..96522918 100644
--- a/gui/modules/Makefile.am
+++ b/gui/modules/Makefile.am
@@ -3,14 +3,17 @@
 INCLUDES = \
 	-I.						\
 	-I..						\
-	-DG_DISABLE_DEPRECATED				\
-	-DGDK_DISABLE_DEPRECATED			\
-	-DGDK_PIXBUF_DISABLE_DEPRECATED			\
-	-DGTK_DISABLE_DEPRECATED			\
-	-DGNOME_DISABLE_DEPRECATED			\
 	-DGNOMELOCALEDIR=\""$(datadir)/locale"\" 	\
 	$(GUI_CFLAGS)
 
+#
+#	-DG_DISABLE_DEPRECATED				\
+#	-DGDK_DISABLE_DEPRECATED			\
+#	-DGDK_PIXBUF_DISABLE_DEPRECATED			\
+#	-DGTK_DISABLE_DEPRECATED			\
+#	-DGNOME_DISABLE_DEPRECATED			\
+#
+
 libkeymouselistener_la_SOURCES = \
 	keymouselistener.c