ListArc » English » Miscellaneous » Pkg-shadow-devel » 4.1.5:+su:+problem+when+there+is+no+controlling+term


4.1.5:+su:+problem+w...


20-02-2012 11:45 AM
1


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

_______________________________________________
Pkg-shadow-devel mailing list
Pkg-shadow-
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-shadow-devel
)



20-05-2012 05:14 PM
2


Hello,

Thanks for your report.

On Mon, Feb 20, 2012 at 12:45:59PM +0100, wrote:
>
[...]
>
> 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:
[...]

I have applied a different version with the same intent:

--- src/su.c
+++ src/su.c
@@ -1092,6 +1092,9 @@
if (fd >= 0) {
err = ioctl (fd, TIOCNOTTY, (char *) 0);
(void) close (fd);
+ } else if (ENXIO == errno) {
+ /* There are no controlling terminal already */
+ err = 0;
}
#endif /* USE_PAM */

(i.e. do not fail if open fails to open /dev/tty because the terminal does
not exist)

In case of PAM, setsid is used, and behaves correctly in such case.

Best Regards,
--
Nekral

_______________________________________________
Pkg-shadow-devel mailing list
Pkg-shadow-
http://lists.alioth.debian.org/cgi-bin/mailman/listinfo/pkg-shadow-devel
)