[pkg-fso-maint] Bug#531194: Bug#531194: nodm: .xsession-errors grows and grows

[Enrico Zini]

On Sat, May 30, 2009 at 06:35:57PM +0200, arne anka wrote:

> just recently i discovered that my ~/.xsession-errors has been growing
> to several mb of size.
> on my desktop computers .xsession-errors never gets that big -- though i
> am tm not sure if it is created from scratch on login or deleted when
> exceeding a limit.
> anyway, since the most notable difference between my desktops and the fr
> nodm vs kdm is, i guess, nodm is responsible for not cleaning out
> ..xsession-errors.
[...]
> no clue. inhowfar that would fit into nodm (and where, nodm_session?).

I agree, I think it fits into the session part of nodm, after it changed
the uid.

Follows the patch that I've prepared and tested. I'm quite happy with
it, and the only doubt that I have is if I should create it passing 0666
to open (therefore leaving the rights up to the umask) or if I should
pass 0600. ~/.xsession-errors is currently world-readable on my laptop
and 0600 on the openmoko, how is it in your system?

I would tend to prefer 0600 because I cannot think of a case why anyone
else would need to read my ~/.xsessione-errors, while I can think of a
few cases where I would not want people to read it, as some application
may spit some sensitive information to stderr. I'd like to hear your
comments.

The proposed patch:

diff --git a/nodm.c b/nodm.c
index 3101448..b015c36 100644
--- a/nodm.c
+++ b/nodm.c
@@ -63,10 +63,12 @@
 #include <string.h>
 #include <errno.h>
 #include <sys/types.h>
+#include <sys/stat.h>
 #include <sys/wait.h>
 #include <syslog.h>
 #include <time.h>
 #include <unistd.h>
+#include <fcntl.h>
 
 #include <security/pam_appl.h>
 #include <security/pam_misc.h>
@@ -193,6 +195,59 @@ int change_uid (const struct passwd *info)
 	return 0;
 }
 
+/*
+ * Cleanup ~/.xsession-errors.
+ *
+ * The function looks for .xsession-errors in the current directory, so when it
+ * is called the current directory must be the user's homedir.
+ *
+ * The function also assumes that we are running as the user. As a consequence
+ * it does not worry about symlink attacks, because they would only be possible
+ * if the user's home directory is group or world writable.
+ *
+ * curdirname is the name of the current directory, and it is only used when
+ * logging error messages.
+ *
+ * The function returns true on success, false on failure.
+ */
+int cleanup_xse(off_t maxsize, const char* curdirname)
+{
+	int ret = 0;
+	int xse_fd = -1;
+	struct stat xse_st;
+
+	xse_fd = open(".xsession-errors", O_WRONLY | O_CREAT, 0666);
+	if (xse_fd < 0)
+	{
+		perror ("open ~/.xsession-errors");
+		syslog (LOG_ERR, "cannot open `%s/%s': %m\n", curdirname, ".xsession-errors");
+		goto cleanup;
+	}
+	if (fstat(xse_fd, &xse_st) < 0)
+	{
+		perror ("stat ~/.xsession-errors");
+		syslog (LOG_ERR, "cannot stat `%s/%s': %m\n", curdirname, ".xsession-errors");
+		goto cleanup;
+	}
+	if (xse_st.st_size > maxsize)
+	{
+		if (ftruncate(xse_fd, 0) < 0)
+		{
+			perror ("truncating ~/.xsession-errors");
+			syslog (LOG_ERR, "cannot truncate `%s/%s': %m\n", curdirname, ".xsession-errors");
+			goto cleanup;
+		}
+	}
+
+	/* If we made it so far, we succeeded */
+	ret = 1;
+
+cleanup:
+	if (xse_fd >= 0)
+		close(xse_fd);
+	return ret;
+}
+
 /* Signal handler for parent process later */
 static void catch_signals (int sig)
 {
@@ -643,7 +698,9 @@ static int nodm_session(int argc, char **argv)
 	unsetenv("NODM_MIN_SESSION_TIME");
 	unsetenv("NODM_RUN_SESSION");
 
-	chdir (pwent.pw_dir);
+	if (chdir (pwent.pw_dir) == 0)
+		/* Truncate ~/.xsession-errors */
+		cleanup_xse(0, pwent.pw_dir);
 
 	args[0] = "/bin/sh";
 	args[1] = "-l";


Ciao,

Enrico

-- 
GPG key: 4096R/E7AD5568 2009-05-08 Enrico Zini <enrico at enricozini.org>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 489 bytes
Desc: Digital signature
URL: <http://lists.alioth.debian.org/pipermail/pkg-fso-maint/attachments/20090706/c5f271ff/attachment.pgp>