Package: cron
Version: 3.0pl1-149
Severity: grave
Tags: security
Justification: user security hole
X-Debbugs-Cc: georg...@debian.org
Hi,
Both setuid() and setgid() return values are not checked in cron's code used to
execute user-provided commands:
do_command.c:
> 63 static void
> 64 child_process(entry *e, user *u) {
> ...
> 243 setgid(e->pwd->pw_gid);
> 244 initgroups(usernm, e->pwd->pw_gid);
> 245 #if (defined(BSD)) && (BSD >= 199103)
> 246 setlogin(usernm);
> 247 #endif /* BSD */
> 248 setuid(e->pwd->pw_uid); /* we aren't root after this... */
> 249
> 250 #endif /* LOGIN_CAP */
man 2 setuid() states the following:
> RETURN VALUE
> On success, zero is returned. On error, -1 is returned, and errno is
> set to indicate the error.
>
> Note: there are cases where setuid() can fail even when the caller is
> UID 0; it is a grave security error to omit checking for a failure return
> from setuid().
In the unlikely event where setuid() (or setgid()) fails, privileges of cron
would not be dropped and commands would be run as root.
This would lead to privilege escalation.
The attached patch fixes this by aborting execution when such an event occurs.
Regards,
-- Package-specific info:
--- EDITOR:
--- /usr/bin/editor:
/usr/bin/nano
--- /usr/bin/crontab:
-rwxr-sr-x 1 root crontab 43648 Jul 25 2022 /usr/bin/crontab
--- /var/spool/cron:
drwxr-xr-x 5 root root 4096 Jun 27 17:17 /var/spool/cron
--- /var/spool/cron/crontabs:
drwx-wx--T 2 root crontab 4096 Jul 25 2022 /var/spool/cron/crontabs
--- /etc/cron.d:
drwxr-xr-x 2 root root 4096 Jun 29 15:08 /etc/cron.d
--- /etc/cron.daily:
drwxr-xr-x 2 root root 4096 Jun 16 17:34 /etc/cron.daily
--- /etc/cron.hourly:
drwxr-xr-x 2 root root 4096 Aug 8 2022 /etc/cron.hourly
--- /etc/cron.monthly:
drwxr-xr-x 2 root root 4096 Nov 30 2022 /etc/cron.monthly
--- /etc/cron.weekly:
drwxr-xr-x 2 root root 4096 Oct 30 2022 /etc/cron.weekly
-- System Information:
Distributor ID: Kali
Description:Kali GNU/Linux Rolling
Release:2022.3
Codename: kali-rolling
Architecture: x86_64
Kernel: Linux 5.18.0-kali5-amd64 (SMP w/4 CPU threads; PREEMPT)
Kernel taint flags: TAINT_UNSIGNED_MODULE
Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE not set
Shell: /bin/sh linked to /usr/bin/dash
Init: systemd (via /run/systemd/system)
LSM: AppArmor: enabled
Versions of packages cron depends on:
ii cron-daemon-common 3.0pl1-149
ii init-system-helpers 1.64+kali2
ii libc62.36-9
ii libpam-runtime 1.5.2-6
ii libpam0g 1.5.2-6
ii libselinux1 3.4-1+b5
ii lsb-base 11.2
ii sensible-utils 0.0.17
Versions of packages cron recommends:
ii exim4-daemon-light [mail-transport-agent] 4.96-14
Versions of packages cron suggests:
pn anacron
pn checksecurity
ii logrotate 3.20.1-1
Versions of packages cron is related to:
pn libnss-ldap
pn libnss-ldapd
pn libpam-ldap
pn libpam-mount
pn nis
pn nscd
-- no debconf information
>From 42309c1fdcc192f356c84221954331b4e64be29e Mon Sep 17 00:00:00 2001
From: Jeffrey Bencteux
Date: Fri, 1 Dec 2023 12:27:21 +0100
Subject: [PATCH] fix unchecked set*id() return values
In the unlikely event where setuid() (or setgid()) fails, privileges of cron
would not be dropped and commands would be run as root.
This would lead to privilege escalation. The below patch fixes this by aborting
execution when such an event occurs.
Signed-off-by: Jeffrey Bencteux
---
do_command.c | 18 +++---
1 file changed, 15 insertions(+), 3 deletions(-)
diff --git a/do_command.c b/do_command.c
index 4083c32..db5227f 100644
--- a/do_command.c
+++ b/do_command.c
@@ -28,7 +28,7 @@ static char rcsid[] = "$Id: do_command.c,v 2.12 1994/01/15
20:43:43 vixie Exp $"
#if defined(SYSLOG)
# include
#endif
-
+#include
static voidchild_process __P((entry *, user *)),
do_univ __P((user *));
@@ -206,11 +206,23 @@ child_process(e, u)
/* set our directory, uid and gid. Set gid first, since once
* we set uid, we've lost root privledges.
*/
- setgid(e->gid);
+ if (setgid(e->gid) == -1)
+ {
+ fprintf(stderr,
+ "could not drop privileges, setgid() failed: %s",
+ strerror(errno));
+ _exit(ERROR_EXIT);
+ }
# if defined(BSD)
initgroups(env_get("LOGNAME", e->envp), e->gid);
# endif
- setuid(e->uid); /* we aren't root after this... */
+ if (setuid(e->uid) == -1) /* we aren't root after
this... */
+ {
+ fprintf(stderr,
+ "could not drop privileges, setuid() failed: %s",
+