[Pkg-shadow-devel] 4.1.5: su: problem when there is no controlling term

[balducci at units.it]

Hello,

I sent this directly to nicolas, but maybe it is worth resending it to the
whole list, as I have seen a report which might deal with the same issue:
(http://lists.alioth.debian.org/pipermail/pkg-shadow-devel/2012-February/009149.html)

The 4.1.4.3 --> 4.1.5 upgrade has this change for su:

  * Do not forward the controlling terminal to commands executed with -c.
    This prevents tty hijacking which could lead to execution with the
    caller's privileges.

Looking at the su.c source, it seems to me that the case when su is run
by a process *WITHOUT* a controlling terminal (e.g. a cron process, or
an init script in sysvinit [see ref above]) is not covered.

Actually, I use su (with -c) from a cron script which, after 4.1.4.3
--> 4.1.5 upgrade, did not work any more (I have been using it for
years without problems).

In su.c, if su is run by a process without controlling terminal (like
cron), then the `open("/dev/tty",O_RDWR)' call will return a negative
fd and the `if (-1 == err)' below will exit giving the error:

     Cannot drop the controlling terminal

(which I find in the output of my script).


Now: if su has not a controlling terminal, it should simply go on
without worrying about dropping it, no?

As a matter of fact, this patch fixes everything for me:

--- shadow-4.1.5/src/su.c.ORIG	2012-02-15 14:22:50.000000000 +0100
+++ shadow-4.1.5/src/su.c	2012-02-15 15:41:14.000000000 +0100
@@ -1089,10 +1089,12 @@
 		/* Otherwise, we cannot use setsid */
 		int fd = open ("/dev/tty", O_RDWR);
 
+                if ((isatty(fd)) == 1) {
 		if (fd >= 0) {
 			err = ioctl (fd, TIOCNOTTY, (char *) 0);
 			(void) close (fd);
 		}
+                } else {err=0;} /* we haven't a controlling terminal at all */
 #endif				/* USE_PAM */
 
 		if (-1 == err) {


This is only a workaround (I do not use PAM: something
similar should be done also for the USE_PAM case), but, in any case, I
think that the current (i.e. 4.1.5) behavior of su should be fixed for
the above scenario in some way.

I report this in the hope to be of any use 

And, of course, thank you very much indeed to the maintainers for your
valuable work and for making it freely available!


ciao
gabriele