Commit Graph

111 Commits

Author SHA1 Message Date
7d82f203ee Reset pid_child only if waitpid was successful.
Do not reset the pid_child to 0 if the child process is still
running. This else-condition can be reached with pid being -1,
therefore explicitly test this condition.

This is a regression fix for CVE-2017-2616. If su receives a
signal like SIGTERM, it is not propagated to the child.

Reported-by: Radu Duta <raduduta@gmail.com>
Signed-off-by: Tobias Stoeckmann <tobias@stoeckmann.org>
2017-05-14 17:58:10 +02:00
08fd4b69e8 su: properly clear child PID
If su is compiled with PAM support, it is possible for any local user
to send SIGKILL to other processes with root privileges. There are
only two conditions. First, the user must be able to perform su with
a successful login. This does NOT have to be the root user, even using
su with the same id is enough, e.g. "su $(whoami)". Second, SIGKILL
can only be sent to processes which were executed after the su process.
It is not possible to send SIGKILL to processes which were already
running. I consider this as a security vulnerability, because I was
able to write a proof of concept which unlocked a screen saver of
another user this way.
2017-02-23 09:47:29 -06:00
a3bf32fe87 shadow: Add auditing support to su
This patch extends the auditing feature used in login to su.

Signed-off-by: Jussi Ohenoja <jussi.ohenoja@nokia.com>
2016-12-13 18:44:19 +02:00
924cc34647 Use sizeof rather than hardcoding snprintf's size argument. 2016-09-20 08:04:14 +01:00
67d2bb6e0a su.c: fix missing length argument to snprintf 2016-09-18 21:31:18 -05:00
5b017af90d Merge pull request #29 from hemio-ev/lower-su-syslog-priority
Reduces syslog priority of common usage events
2016-08-05 11:59:15 -05:00
61fc90b268 Reduces syslog priority of common usage events
- Log INFO instead of ERR on `su missing-user`
- Log NOTICE/WARN instead of ERR on pam_authenticate failure (wrong password for example)
2016-08-04 22:17:31 +02:00
dd50014055 Fixed signal races in shadow tools.
Some of the supplied tools use functions which are not signal-safe.

Most of the times it's exit() vs. _exit().

In other times it's how the standard output or standard error is
handled. FILE-related functions shall be avoided, therefore I replaced
them with write().

Also there is no need to call closelog(). At worst, it allows to
trigger a deadlock by issuing different signal types at bad timings.
But as these fixes are about race conditions, expect bad timings in
general for these bugs to be triggered. :)
2016-07-02 18:11:09 +02:00
00f573fce2 Fix copyright dates. 2013-08-13 23:13:26 +02:00
8781aff637 Terminate the child before closing the PAM session.
* src/su.c: Terminate the child (if needed) before closing the PAM
	session. This is probably more correct, and avoid reporting
	termination from signals possibly sent by PAM modules (e.g. former
	versions of pam_systemd). Debian#670132
2013-08-13 19:48:53 +02:00
a5e3dbb0e3 Reset caught variable when signal is handled by su.
* src/su.c: When a SIGTSTP is caught, reset caught to 0. There is
	no need to kill the child in such case after su is resumed. This
	remove the "Session terminated, terminating shell...
	...terminated." messages in such case.
2013-08-13 19:42:50 +02:00
d172cccd07 Kill the child process group, rather than just the immediate child;
this is needed now that su no longer starts a controlling terminal
when not running an interactive shell (closes: Debian#713979)
2013-07-28 14:38:12 +02:00
9151e673e4 * NEWS: Set release date. 2012-05-25 11:51:53 +00:00
f100b5ea7e * src/su.c: non PAM enabled versions: do not fail if su is called
without a controlling terminal. Ignore ENXIO errors when opening
	/dev/tty.
2012-05-20 16:15:14 +00:00
f0a63185c9 * src/chage.c, src/chgpasswd.c, src/chpasswd.c, src/chsh.c,
src/faillog.c, src/gpasswd.c, src/groupadd.c, src/groupdel.c,
	src/groupmems.c, src/groupmod.c, src/grpconv.c, src/grpunconv.c,
	src/lastlog.c, src/newusers.c, src/passwd.c, src/pwconv.c,
	src/pwunconv.c, src/su.c, src/useradd.c, src/userdel.c,
	src/usermod.c, src/vipw.c: Align and sort options.
2011-11-06 18:39:59 +00:00
6eb0500d3d * src/faillog.c, src/chage.c, src/newusers.c, src/su.c: The getopt
index of long options is not used.
2011-11-06 18:38:45 +00:00
6f05b866bc * src/su.c: Too much const were added on 2011-08-15. pw in
save_caller_context() is allocated and freed.
	* src/su.c: Added missing #endif indication
	* src/su.c save_caller_context(): password only needed if
	SU_ACCESS and !USE_PAM.
2011-09-18 17:47:03 +00:00
5eb9ed0aaf * src/su.c: Added const modifiers.
* lib/prototypes: Synchronize splint annotations.
2011-08-15 09:25:58 +00:00
94c1763f71 * src/su.c: Add splint annotations.
* src/su.c: Set caller_on_console as boolean.
	* src/su.c: Ignore retunr value from fputs (usage) / puts (prompt).
	* src/su.c: Improved memory management.
2011-08-14 21:44:46 +00:00
745bcb5406 * src/su.c: Add annotations to indicate that su_failure() does
not return.
2011-08-14 13:15:20 +00:00
907025eb40 * src/su.c: environ is provided by <unistd.h>.
* src/su.c: Added function prototypes.
	* src/su.c: Rename shellstr parameter to shellname to avoid
	collision with static variable.
	* NEWS, src/su.c: Added support for PAM modules which change
	PAM_USER.
2011-06-16 21:21:29 +00:00
66d71aafb7 Cleanup. 2011-06-13 18:27:51 +00:00
317939e821 * src/su.c: After prepare_pam_close_session() there is no need to
close the session in the child. Added pam_setcred to
	prepare_pam_close_session().
2011-06-13 18:27:46 +00:00
1340beed16 * src/su.c: Also drop the controlling terminal when PAM is not
used.
	* src/su.c: Remove run_shell().
2011-06-13 18:27:40 +00:00
e9045e9f55 * src/su.c (prepare_pam_close_session): Extract the creation of a
child and listening for signal in the parent from run_shell().
	prepare_pam_close_session() is now executed before the creation of
	the pam session and before the UID is changed. This allows to
	close the session as root.
2011-06-13 18:27:34 +00:00
69371ba2c2 * src/su.c: Extract export of environment from main(). 2011-06-13 18:27:28 +00:00
dbe0b96404 * src/su.c: Extract command line processing from main(). 2011-06-13 18:27:23 +00:00
43e65fdd46 * src/su.c: Close the password databases together with syslog. 2011-06-13 18:27:17 +00:00
5d6f4f8ea4 * src/su.c: Merge environment setting blocks after the creation of
the session.
2011-06-13 18:27:12 +00:00
b63c830547 * src/su.c: Cleanup check_perms_nopam(). 2011-06-13 18:27:06 +00:00
927c2f06a3 * src/su.c: Split check_perms() in to a PAM (check_perms_pam) and
a non-PAM (check_perms_nopam) version.
2011-06-13 18:26:58 +00:00
40a16a1b04 Compile fixes & cleanups. 2011-06-13 18:26:52 +00:00
6be3ac560b * src/su.c (save_caller_context): Extract from main() the code
used to save the caller's context.
2011-06-13 18:26:47 +00:00
b661b913b5 * src/su.c: Cleanup local variables. 2011-06-13 18:26:36 +00:00
b273c4d19f Cleanup comments. 2011-06-13 18:26:31 +00:00
7ebfb5c90f * lib/prototypes.h, src/suauth.c, src/su.c (check_su_auth): Do not
use the pwent global variable to communicate between APIs of
	different files. Added boolean parameter su_to_root to
	check_su_auth().
	* src/su.c (check_perms): Return the passwd entry of the finally
	authenticated user. Remove usage of the pwent variable.
	* src/su.c: The password of the caller is the one from the
	spwd structure only if the passwd's password is 'x'.
2011-06-13 18:26:26 +00:00
3abd71c526 * src/su.c: Extract the authentication from the main function. 2011-06-13 18:26:16 +00:00
dc90c77285 * src/su.c: Define shellstr before the environment so that
restricted_shell is called only once. This will allow moving the
	environment definition after the switch to the new user.
2011-06-13 18:26:10 +00:00
2f71935616 * src/su.c: Move definition of change_environment and shellstr
after the switch to the final subsystem. The previous architecture
	forced to always change the environment (the shell starts with a
	'*' and was thus restricted, and change_environment could not be
	reset to false).
2011-06-13 18:25:57 +00:00
941e94f1d2 * src/su.c: No need to change the user's shell in case of
subsystem root. Update the comments.
2011-06-13 18:25:51 +00:00
4be8077a56 * src/su.c: Group some of the environment processing blocks. The
definition of shellstr, PATH and IFS is not influenced (getenv,
	getdef, restricted_shell) by and does not influence (addenv does
	not change environ) the authentication. And the authentication
	does not overwrite those definitions. This will ease an extraction
	from the big main() function.
2011-06-13 18:25:45 +00:00
aef33a89c2 * src/su.c: No need to call expire() if there are no shadow entry.
No need to convert a passwd entry into an spwd entry in that case.
2011-06-13 18:25:40 +00:00
1ddde00b1d * src/su.c: Updating pwent after expire() is not useful. Only the
password information may have changed and they are not used
	anymore afterwards.
2011-06-13 18:25:34 +00:00
b5403415f7 * NEWS, src/su.c: 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. This required to
	forward signals from the terminal (SIGINT, SIGQUIT, SIGTSTP) to
	the executed command.
2011-06-05 14:41:15 +00:00
991ce97170 * src/su.c: Fix handling of environment variables when
the environment is not changed. In particular, this makes su
	behave as documented regarding PATH and IFS (i.e. they are reset)
	when -p is provided.
2010-08-28 19:55:31 +00:00
ab9427420e * src/newgrp.c, src/userdel.c, src/grpck.c, src/gpasswd.c,
src/newusers.c, src/pwconv.c, src/chpasswd.c, src/logoutd.c,
	src/chfn.c, src/groupmems.c, src/usermod.c, src/pwunconv.c,
	src/expiry.c, src/groupdel.c, src/chgpasswd.c, src/useradd.c,
	src/su.c, src/groupmod.c, src/passwd.c, src/pwck.c, src/chage.c,
	src/groupadd.c, src/login.c, src/grpconv.c, src/groups.c,
	src/grpunconv.c, src/chsh.c: Prog changed to a constant string.
2010-08-22 19:36:09 +00:00
07e462f01f * libmisc/limits.c: Avoid implicit conversion of integer to
boolean.
	* libmisc/basename.c: Avoid implicit conversion of pointer to
	boolean.
	* libmisc/basename.c, lib/prototypes.h (Basename): Return a
	constant string.
	* libmisc/basename.c, libmisc/obscure.c, lib/prototypes.h,
	libmisc/xmalloc.c, libmisc/getdate.h, libmisc/system.c,
	libmisc/getgr_nam_gid.c, libmisc/failure.c, libmisc/valid.c: Add
	splint annotations.
	* libmisc/chowndir.c: Avoid memory leak.
	* libmisc/chowndir.c: Do not check *printf/*puts return value.
	* libmisc/chowntty.c: Avoid implicit conversion between integer
	types.
	* libmisc/obscure.c: Return a bool when possible instead of int.
	* libmisc/shell.c: Do not check *printf/*puts return value.
	* libmisc/shell.c: Do not check execle return value.
	* libmisc/setupenv.c: Avoid implicit conversion between integer
	types.
	* libmisc/xmalloc.c: size should not be zero to avoid returning
	NULL pointers.
	* libmisc/hushed.c: Do not check *printf/*puts return value.
	* libmisc/system.c: Avoid implicit conversion of integer to
	boolean. safe_system last argument is a boolean.
	* libmisc/system.c: Check return value of dup2.
	* libmisc/system.c: Do not check *printf/*puts return value.
	* libmisc/system.c: Do not check execve return value. 
	* libmisc/salt.c: Do not check *printf/*puts return value.
	* libmisc/loginprompt.c: Do not check gethostname return value.
	* libmisc/find_new_gid.c, libmisc/find_new_uid.c: Do not check
	gr_rewind/pw_rewind return value.
	* libmisc/ttytype.c: Limit the number of parsed characters in the
	sscanf format.
	* libmisc/ttytype.c: Test if a type was really read.
	* libmisc/sub.c: Do not check *printf/*puts return value.
	* libmisc/sub.c: Avoid implicit conversion of integer to boolean.
	* src/userdel.c: Fix typo in comment.
	* src/userdel.c: Avoid implicit conversion of boolean to integer.
	* src/userdel.c: safe_system last argument is a boolean.
	* src/newusers.c: Avoid implicit conversion of boolean to integer.
	* src/newusers.c: Avoid implicit conversion of integer to boolean.
	* src/usermod.c: Add brackets.
	* src/usermod.c: Avoid implicit conversion of characters or
	integers to booleans.
	* src/vipw.c: Avoid implicit conversion of integer to boolean.
	* src/su.c: Avoid implicit conversion of integer to boolean.
	* src/su.c: Add brackets.
	* src/useradd.c: Avoid implicit conversion of characters or
	integers to booleans.
2010-08-22 19:13:53 +00:00
7eb6a4b3a4 Updated copyrights. 2010-08-22 13:04:54 +00:00
471a2df3a6 * libmisc/obscure.c, lib/prototypes.h (obscure): Return a bool
instead of an int.
	* libmisc/obscure.c, libmisc/tz.c, src/passwd.c, lib/encrypt.c,
	libmisc/copydir.c, lib/prototypes.h: Add splint annotations.
	* libmisc/tz.c: Fix some const issues.
	* libmisc/tz.c: Avoid multi-statements lines.
	* libmisc/tz.c: Add brackets.
	* libmisc/copydir.c: Do not check *printf/*puts return value.
	* libmisc/copydir.c: Fail if we cannot set or reset the SELinux
	fscreate context.
	* libmisc/copydir.c: Use xmalloc instead of malloc.
	* libmisc/copydir.c: Do not check lutimes return value
	* src/vipw.c: Avoid implicit conversion of integer to boolean.
	* src/su.c (iswheel): Return a bool instead of an int.
	* src/passwd.c: Remove insert_crypt_passwd(). Use xstrdup instead.
	* src/passwd.c: Return constant strings when sufficient.
	* src/passwd.c: Do not check *printf/*puts return value.
	* src/passwd.c: Avoid implicit conversion of character to boolean.
	* src/passwd.c: Do not check sleep return value.
	* src/sulogin.c: Do not check *printf/*puts return value.
	* lib/encrypt.c: Do not check fprintf return value.
2010-08-22 12:49:07 +00:00
427b60f288 * src/su.c: shell's name must be -su when a su fakes a login. 2010-03-30 21:32:36 +00:00