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  libc6                2.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        <none>
pn  checksecurity  <none>
ii  logrotate      3.20.1-1

Versions of packages cron is related to:
pn  libnss-ldap   <none>
pn  libnss-ldapd  <none>
pn  libpam-ldap   <none>
pn  libpam-mount  <none>
pn  nis           <none>
pn  nscd          <none>

-- no debconf information
>From 42309c1fdcc192f356c84221954331b4e64be29e Mon Sep 17 00:00:00 2001
From: Jeffrey Bencteux <jeffbenct...@gmail.com>
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 <jeffbenct...@gmail.com>
---
 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 <syslog.h>
 #endif
-
+#include <errno.h>
 
 static void            child_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",
+                       strerror(errno));
+                       _exit(ERROR_EXIT);
+               }
                chdir(env_get("HOME", e->envp));
 
                /* exec the command.
-- 
2.35.1

Reply via email to