Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 197625 - sys-process/vixie-cron - add "_ +" to safe characters
Summary: sys-process/vixie-cron - add "_ +" to safe characters
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Current packages (show other bugs)
Hardware: All All
: High enhancement with 3 votes (vote)
Deadline: 2019-10-11
Assignee: No maintainer - Look at if you want to take care of it
Keywords: PMASKED
: 247179 328569 527102 (view as bug list)
Depends on:
Reported: 2007-10-31 12:24 UTC by Miloslav Semler
Modified: 2019-10-11 15:08 UTC (History)
11 users (show)

See Also:
Package list:
Runtime testing required: ---


Note You need to log in before you can comment on or make changes to this bug.
Description Miloslav Semler 2007-10-31 12:24:04 UTC
Recently I solved why cron reports "UNSAFE" to 
syslog and does not send mails. Some usernames on my server contain underscore
character. So I think this is safe character - it could be added to 

Miloslav Semler

--- do_command.c.orig   2007-10-30 19:47:09.000000000 +0100
+++ do_command.c        2007-10-30 19:47:25.000000000 +0100
@@ -511,7 +511,7 @@ child_process(entry *e, user *u) {

 static int
 safe_p(const char *usernm, const char *s) {
-       static const char safe_delim[] = "@!:%-.,";     /* conservative! */
+       static const char safe_delim[] = "_@!:%-.,";     /* conservative! */
        const char *t;
        int ch, first;
Comment 1 Thilo Bangert (RETIRED) (RETIRED) gentoo-dev 2008-10-08 14:20:39 UTC
security: can you please comment on the safeness of '_' as a username delimiter.

Comment 2 Christian Hoffmann (RETIRED) gentoo-dev 2008-10-08 16:13:31 UTC
This code snippet is strange. _ is a valid char in usernames, as I got it (and as the submitter of this bug said), but it's certainly not a valid username *delimiter*. So, semantically, this patch is wrong.
Looking at the code [1], the function safe_p() is apparently used to validate email addresses (and yes, "username" is a valid email address as well). If this is the only way of calling of this function, this should be fine.

It would be more correct to do sth like:
  isalnum(ch) || t == "_" || (!first ...)

Not sure if this is syntactically correct (I'm not really a C coder), but semantically it should be, IMO.

Not speaking for security here though.

Comment 3 Miloslav Semler 2008-10-10 15:51:51 UTC
If you browse the code, you notice that this fn (safe_p) checks only mailto. So this variable is used as definition of allowed non-alphanumeric characters in mail. Maybe it would be better rename safe_delim to special_allowed. Also notice that if you run cron in non-c locale, you can get so characters as ěščřžý... So maybe this would be more accurate:

--- do_command.c.orig   2007-10-30 19:47:09.000000000 +0100
+++ do_command.c        2008-10-10 17:42:54.000000000 +0200
@@ -511,14 +511,9 @@

 static int
 safe_p(const char *usernm, const char *s) {
-       static const char safe_delim[] = "@!:%-.,";     /* conservative! */
-       const char *t;
-       int ch, first;
+       static const char allowed[] = "0123456789abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ_@!:%-.,";     /* conservative! */

-       for (t = s, first = 1; (ch = *t++) != '\0'; first = 0) {
-               if (isascii(ch) && isprint(ch) &&
-                   (isalnum(ch) || (!first && strchr(safe_delim, ch))))
-                       continue;
+       if(strcspn(s, allowed) != strlen(s)){
                log_it(usernm, getpid(), "UNSAFE", s);
                return (FALSE);
Comment 4 Thilo Bangert (RETIRED) (RETIRED) gentoo-dev 2009-01-31 14:15:16 UTC
*** Bug 247179 has been marked as a duplicate of this bug. ***
Comment 5 Thilo Bangert (RETIRED) (RETIRED) gentoo-dev 2009-05-14 16:48:34 UTC
cronie - a fork of vixie-cron - has this change in its vcs.;a=commit;h=080416de55c57d3638769ebfb30dde828bfb7e95

Comment 6 Rob Rosenfeld 2009-07-10 03:32:14 UTC
The plus sign, '+', is also valid in email addresses for plussed detail.  

RFC 5322 & RFC3696
Comment 7 Chris Gianelloni 2009-08-31 19:40:04 UTC
After the comments here, this looks like something that should be fixed.  Who is responsible for getting this done in the ebuild?  Does this need to pass to the QA team?
Comment 8 Nathan March 2009-09-16 21:57:34 UTC
There's an additional bug in this code, if mailto is unsafe the file handle is opened but the code still tries to write to it and close it later. End result is cron segfaulting....

To reproduce just set MAILTO to something like "," and set a cron to echo "test". If you strace or have grsec, you'll see cron segfaulting after it dumps the unsafe line to the log.

Patch to fix:

--- do_command.c.orig   2009-09-16 14:49:58.000000000 -0700
+++ do_command.c        2009-09-16 14:48:16.000000000 -0700
@@ -445,6 +445,10 @@
                                /* this was the first char from the pipe
                                putc(ch, mail);
+                       } else {
+                               /*  Mailto has to be reset to null otherwise later calls will try to use the file handle
+                                */
+                               mailto = NULL;
                        /* we have to read the input pipe no matter whether
Comment 9 Thilo Bangert (RETIRED) (RETIRED) gentoo-dev 2009-09-18 18:48:00 UTC
cronie has fixed this differently, see;a=commitdiff;h=cd701fdd9df178112e049d166405462933a3b987
Comment 10 lyle2.0 2010-06-29 14:28:13 UTC
(In reply to comment #6)
> The plus sign, '+', is also valid in email addresses for plussed detail.  
> RFC 5322 & RFC3696

This is how I discovered the issue. The manual only refers to MAILTO referencing a *user*; is it also safe to assume it accepts an email address?
Comment 11 David Voge 2011-06-10 13:13:23 UTC
What is the current state? Anyone an idea?
Comment 12 floppe 2012-05-18 05:22:45 UTC
Still segfaults with plus sign so no cron jobs run. Ticket importance too low maybe when segfaults are involved?
Comment 13 Paul B. Henson 2012-05-18 05:33:31 UTC
We switched to cronie and haven't looked back. Maybe it's time to get rid of vixie-cron, which I don't believe is maintained anymore?
Comment 14 Philippe Chaintreuil 2013-12-10 14:39:54 UTC
I'm getting these warnings and segfaults as well.

If switching to cronie is the suggested approach, the handbook should be updated as well, since it's telling everyone that vixie is preferred.
Comment 15 Jeroen Roovers (RETIRED) gentoo-dev 2014-10-27 17:01:51 UTC
*** Bug 328569 has been marked as a duplicate of this bug. ***
Comment 16 Jeroen Roovers (RETIRED) gentoo-dev 2014-10-27 17:01:55 UTC
*** Bug 527102 has been marked as a duplicate of this bug. ***
Comment 17 Thomas Witzenrath 2018-07-31 10:23:55 UTC
Bump. It's 2018, this bug has been unfixed for over 10 years now.
Comment 18 Paul B. Henson 2018-08-08 00:48:29 UTC
Is this package even maintained anymore? Why not mask it and have people switch to sys-process/cronie?
Comment 19 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2019-10-11 15:08:01 UTC
Package removed.