Bug#1057265: cron: Uncheck return values of set*id() family functions

2023-12-03 Thread Jeffrey
Hi Christian,

Indeed I was looking at the unpatched version. Sorry for the inconvenience.

Best regards,

-- 
Jeffrey BENCTEUX


Le sam. 2 déc. 2023 à 20:05, Christian Kastner  a écrit :

> Hi Jeffrey,
>
> On 2023-12-02 11:39, Jeffrey Bencteux wrote:
> > Hi,
> >
> > Both setuid() and setgid() return values are not checked in cron's code
> used to execute user-provided commands:
>
> This issue was reported as CVD-2006-2607 and fixed a long time ago.
>
> Here's the relevant patch:
>
>
> https://sources.debian.org/src/cron/3.0pl1-162/debian/patches/fixes/Check-privilege-drop-results-CVE-2006-2607.patch/
>
> Are you perhaps looking at the unpatched source?
>
> Best,
> Christian
>


Bug#1057265: cron: Uncheck return values of set*id() family functions

2023-12-02 Thread Christian Kastner
Hi Jeffrey,

On 2023-12-02 11:39, Jeffrey Bencteux wrote:
> Hi,
> 
> Both setuid() and setgid() return values are not checked in cron's code used 
> to execute user-provided commands:

This issue was reported as CVD-2006-2607 and fixed a long time ago.

Here's the relevant patch:

https://sources.debian.org/src/cron/3.0pl1-162/debian/patches/fixes/Check-privilege-drop-results-CVE-2006-2607.patch/

Are you perhaps looking at the unpatched source?

Best,
Christian



Bug#1057265: cron: Uncheck return values of set*id() family functions

2023-12-02 Thread Jeffrey Bencteux
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",
+