[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#256299: xdm: race condition in policy.c:Willing()



Package: xdm
Version: 4.3.0.dfsg.1-5
Severity: important
Tags: upstream

Notes to self:

1) Set submitter to "Chip Coldwell <coldwell@physics.harvard.edu>" after
BTS acks this bug.

2) Kick severity up to RC if we can determine that this race condition
constitutes a security problem.

----- Forwarded message from Chip Coldwell <coldwell@physics.harvard.edu> -----

From: Chip Coldwell <coldwell@physics.harvard.edu>
To: Branden Robinson <branden@debian.org>
Cc: Debian X Strike Force <debian-x@lists.debian.org>
Subject: xdm race condition
Date: Thu, 24 Jun 2004 13:58:06 -0400 (EDT)
Message-ID: <[🔎] Pine.LNX.4.58.0406240946480.19170-200000@physics.harvard.edu>
X-Spam-Status: No, hits=-4.9 required=4.0 tests=BAYES_00 autolearn=ham 
	version=2.63


I found another xdm bug.  This time it's a race condition in 

xc/programs/xdm/policy.c:Willing

around line numbers 140--145, which reads

	    if ((fd = popen(willing, "r")))
	    {
		char *s = NULL;
		while(!(s = fgets(statusBuf, 256, fd)) && errno == EINTR)
			;

Here's the problem.  The "popen" call creates a child process and a
pipe to communicate with it.  If the child process exits during the
"fgets" call without generating any output, the parent process
receives SIGCHLD and the read system call gets interrupted.  Therefore
errno == EINTR, and since the child has exited the pipe never returns
any data.  xdm goes into an infinite loop.  I think the problem is
that fgets doesn't reset errno to zero; we have to do that manually.
The fix is the trivial patch attached to this email.

(The child process is the "Xwilling" script; in the case of the default
Debian configuration it is

"su nobody -c /usr/X11R6/lib/X11/xdm/Xwilling")

Chip

-- 
Charles M. "Chip" Coldwell
System Administrator
Harvard Physics Department
617-495-3388

Content-Description: xdm race condition fix
--- xc/programs/xdm/policy.c~	2002-12-07 15:31:04.000000000 -0500
+++ xc/programs/xdm/policy.c	2004-06-24 09:56:19.000000000 -0400
@@ -140,8 +140,9 @@
 	    if ((fd = popen(willing, "r")))
 	    {
 		char *s = NULL;
+		errno = 0;
 		while(!(s = fgets(statusBuf, 256, fd)) && errno == EINTR)
-			;
+			errno = 0;
 		if (s && strlen(statusBuf) > 0)
 			statusBuf[strlen(statusBuf)-1] = 0; /* chop newline */
 		else


----- End forwarded message -----

-- 
G. Branden Robinson                |      "There is no gravity in space."
Debian GNU/Linux                   |      "Then how could astronauts walk
branden@debian.org                 |       around on the Moon?"
http://people.debian.org/~branden/ |      "Because they wore heavy boots."

Attachment: signature.asc
Description: Digital signature


Reply to: