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 safe_delim. Cheers, 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;
security: can you please comment on the safeness of '_' as a username delimiter. thanks.
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. [1] https://fedorahosted.org/cronie/browser/src/do_command.c
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); }
*** Bug 247179 has been marked as a duplicate of this bug. ***
cronie - a fork of vixie-cron - has this change in its vcs. http://git.fedoraproject.org/git/?p=cronie.git;a=commit;h=080416de55c57d3638769ebfb30dde828bfb7e95 thanks
The plus sign, '+', is also valid in email addresses for plussed detail. RFC 5322 & RFC3696
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?
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 "nathan@whatever.com, nathan@whatever.com" 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
cronie has fixed this differently, see http://git.fedorahosted.org/git/cronie.git?p=cronie.git;a=commitdiff;h=cd701fdd9df178112e049d166405462933a3b987
(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?
What is the current state? Anyone an idea?
Still segfaults with plus sign so no cron jobs run. Ticket importance too low maybe when segfaults are involved?
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?
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.
*** Bug 328569 has been marked as a duplicate of this bug. ***
*** Bug 527102 has been marked as a duplicate of this bug. ***
Bump. It's 2018, this bug has been unfixed for over 10 years now.
Is this package even maintained anymore? Why not mask it and have people switch to sys-process/cronie?
Package removed.
