Package: cron
Version: 3.0pl1-212
Severity: normal
Tags: patch
The final username in an administrator-created /etc/cron.deny is not
matched if that line has no terminating newline. For example, when the
file consists of the bytes "blocked" without a trailing LF, the user
"blocked" can still run crontab(1), assuming no cron.allow file takes
precedence. The administrator expects that username to be denied. This
is conditional on use of cron.deny and the missing final LF; it does
not grant higher OS privileges.
The in_file() reader removes the final character unconditionally as
though it were a newline. The attached Debian -212 patch reads
complete physical records, matches a final unterminated line, handles
CRLF, and rejects overlong or NUL-containing records without consuming
a following valid entry. Its regression test covers those cases.
The patched Debian source built in a disposable trixie container; the
focused regression and all 13 existing Debian integration tests
passed. Ubuntu versions 3.0pl1-200ubuntu1 and -209ubuntu1 have the
same issue. Debian and Ubuntu Security Teams advised public bug
reports for these findings.
Regards,
Brett Sutton
Description: Match complete final lines in cron allow and deny files
Origin: other, private Daybreak audit
Forwarded: no
--- a/misc.c
+++ b/misc.c
@@ -427,9 +427,23 @@
char line[MAX_TEMPSTR];
rewind(file);
- while (fgets(line, MAX_TEMPSTR, file)) {
- if (line[0] != '\0')
- line[strlen(line)-1] = '\0';
+ for (;;) {
+ size_t len = 0;
+ int ch, invalid = 0;
+ /* Consume exactly one physical record, even with embedded NULs. */
+ while ((ch = fgetc(file)) != EOF && ch != '\n') {
+ if (ch == '\0' || len == sizeof line - 1)
+ invalid = 1;
+ else
+ line[len++] = ch;
+ }
+ if (ch == EOF && len == 0)
+ break;
+ if (invalid)
+ continue;
+ line[len] = '\0';
+ if (len && line[len-1] == '\r')
+ line[len-1] = '\0';
if (0 == strcmp(line, string))
return TRUE;
}
@@ -437,6 +451,7 @@
}
+
/* int allowed(char *username)
* returns TRUE if (ALLOW_FILE exists and user is listed)
* or (DENY_FILE exists and user is NOT listed)
--- /dev/null
+++ b/tests/deny_regression.c
@@ -0,0 +1,41 @@
+#define MAIN_PROGRAM
+#include "../misc.c"
+#include <assert.h>
+
+static void
+check_deny_lines(void)
+{
+ FILE *file = tmpfile();
+ char long_name[MAX_TEMPSTR + 10];
+ char truncated[MAX_TEMPSTR];
+ assert(file != NULL);
+ fputs("blocked", file);
+ assert(in_file("blocked", file));
+ freopen(NULL, "w+", file);
+ fputs("blocked\n", file);
+ assert(in_file("blocked", file));
+ freopen(NULL, "w+", file);
+ fputs("blocked\r\n", file);
+ assert(in_file("blocked", file));
+ freopen(NULL, "w+", file);
+ memset(long_name, 'a', sizeof long_name - 1);
+ long_name[sizeof long_name - 1] = '\0';
+ memset(truncated, 'a', sizeof truncated - 1);
+ truncated[sizeof truncated - 1] = '\0';
+ fprintf(file, "%s\nblocked\n", long_name);
+ assert(!in_file(truncated, file));
+ assert(in_file("blocked", file));
+ fclose(file);
+ file = tmpfile();
+ assert(file != NULL);
+ {
+ static const char malformed[] = "junk\0junk\nblocked\n";
+ assert(fwrite(malformed, 1, sizeof malformed - 1, file) == sizeof malformed - 1);
+ }
+ assert(!in_file("junk", file));
+ assert(in_file("blocked", file));
+ fclose(file);
+}
+
+
+int main(void) { check_deny_lines(); return 0; }
