Fix Cron job input validation

This patch fixes unsafe validation of Cron job fields before crontab entries are created.

It rejects line breaks and invalid schedule, special-time, and date values that could add unexpected fields, commands, or environment variables.

It also adds tests for malicious inputs and valid schedules.

Additional improvements to accompany. c23b6b5b48.

cron/cron-lib.pl

@@ -1079,6 +1079,8 @@ if ($in{'special_def'}) {
 	foreach $arr (@pers) {
 		delete($job->{$arr});
 		}
+	# Special times are single words like daily or reboot
+	$in{'special'} =~ /^[a-z]+$/i || &error($text{'save_especial'});
 	$job->{'special'} = $in{'special'};
 	}
 else {
@@ -1092,6 +1094,11 @@ else {
 			# Need to work out and simplify ranges selected
 			local (@range, @newrange, $i);
 			@range = split(/\0/, $in{$arr});
+			# Selected times are plain numbers
+			foreach my $v (@range) {
+				$v =~ /^\d+$/ || &error(&text('save_etime',
+							$text{"edit_$arr"}));
+				}
 			@range = sort { $a <=> $b } @range;
 			local $start = -1;
 			for($i=0; $i<@range; $i++) {
@@ -1168,6 +1175,11 @@ if ($in->{'range_def'}) {
 else {
 	# Validate and store range
 	foreach my $r ("start", "end") {
+		# Dates go into the job's command, so only allow numbers
+		foreach my $f ("day", "month", "year") {
+			$in->{'range_'.$r.'_'.$f} =~ /^\d+$/ ||
+				&error($text{'range_e'.$r});
+			}
 		eval { timelocal(0, 0, 0, $in->{'range_'.$r.'_day'},
 					  $in->{'range_'.$r.'_month'}-1,
 					  $in->{'range_'.$r.'_year'}-1900) };

cron/lang/en

@@ -81,6 +81,9 @@ lcedit_special_reboot=when system boots
 
 save_err=Failed to save cron job
 save_ecmd=You did not enter a command to execute
+save_enewline=The command and comment cannot contain line breaks
+save_especial=Invalid special time selected
+save_etime=Invalid $1 selected
 save_ename=Missing or invalid environment variable name
 save_evalue=Invalid environment variable value
 save_euser=You must select a user

cron/save_cron.cgi

@@ -38,6 +38,10 @@ foreach $f (@files) { &lock_file($f); }
 if ($in{"cmd"} !~ /\S/ && $access{'command'}) {
 	&error($text{'save_ecmd'});
 	}
+# A line break would add extra lines to the crontab
+if ($in{'cmd'} =~ /[\r\n]/ || $in{'comment'} =~ /[\r\n]/) {
+	&error($text{'save_enewline'});
+	}
 if (&supports_users()) {
 	if (!$in{'user'}) {
 		&error($text{'save_euser'});

cron/t/run-tests.t

@@ -81,15 +81,23 @@ ok(!cron_move_affects_env(\@jobs, $jobs[0], $jobs[6]),
 my $cronfile = "$vardir/crontab";
 my $crondir = "$vardir/crontabs";
 make_path($crondir);
-open(my $modfh, ">", "$confdir/cron/config") or die "cron config: $!";
-print $modfh "single_file=$cronfile\n";
-print $modfh "cron_dir=$crondir\n";
-print $modfh "system_crontab=$vardir/system-crontab\n";
-print $modfh "cronfiles_dir=$vardir/cron.d\n";
-print $modfh "vixie_cron=1\n";
-close($modfh);
 make_path("$vardir/cron.d");
 
+# Write the Cron module config, with the given lines added.
+sub write_cron_config
+{
+my (@lines) = @_;
+open(my $fh, ">", "$confdir/cron/config") or die "cron config: $!";
+print $fh map { "$_\n" } (@lines,
+	"cron_dir=$crondir",
+	"system_crontab=$vardir/system-crontab",
+	"cronfiles_dir=$vardir/cron.d",
+	"vixie_cron=1",
+	"cron_deny_all=2");
+close($fh);
+}
+write_cron_config("single_file=$cronfile");
+
 # Reset the crontab fixture to its original contents.
 sub write_crontab
 {
@@ -246,6 +254,39 @@ like($result->{'out'}, qr/^Location:/m,
 is(read_crontab_fixture(), $original,
 	"re-enabling the normal job restores the fixture");
 
+# Job fields must not add lines or fields to the crontab
+seed_acl(%full_acl);
+my %edit = ( 'idx' => 0, 'user' => 'root', 'active' => 1,
+	     'cmd' => 'echo first', 'range_def' => 1,
+	     'special_def' => 0, 'all_mins' => 1, 'all_hours' => 1,
+	     'all_days' => 1, 'all_months' => 1, 'all_weekdays' => 1 );
+foreach my $case (
+	[ "a line break in the command",
+	  { 'cmd' => "echo first\nPATH=/tmp" }, qr/cannot contain line breaks/ ],
+	[ "a line break in the comment",
+	  { 'comment' => "x\nPATH=/tmp" }, qr/cannot contain line breaks/ ],
+	[ "a special time with spaces",
+	  { 'special_def' => 1, 'special' => "reboot echo x #" },
+	  qr/Invalid special time/ ],
+	[ "a minute with spaces",
+	  { 'all_mins' => 0, 'mins' => "* * * * * echo x #" },
+	  qr/Invalid Minutes selected/ ],
+	[ "a minute with a line break",
+	  { 'all_mins' => 0, 'mins' => "5\nPATH=/tmp" },
+	  qr/Invalid Minutes selected/ ],
+	[ "a range date with shell text",
+	  { 'range_def' => 0, 'range_start_day' => '1;echo x;',
+	    'range_start_month' => 1, 'range_start_year' => 2026,
+	    'range_end_day' => 31, 'range_end_month' => 12,
+	    'range_end_year' => 2026 }, qr/invalid start date/i ],
+	) {
+	my ($desc, $form, $re) = @$case;
+	$result = run_cgi("save_cron.cgi", { %edit, %$form });
+	like($result->{'out'}, $re, "save_cron.cgi rejects $desc");
+	is(read_crontab_fixture(), $original,
+		"rejected $desc leaves the crontab unchanged");
+	}
+
 seed_acl(%full_acl);
 $result = run_cgi("move.cgi", { 'idx' => 4, 'top' => 1 });
 like($result->{'out'}, qr/^Location:/m,
@@ -253,4 +294,23 @@ like($result->{'out'}, qr/^Location:/m,
 isnt(read_crontab_fixture(), $original,
 	"the permitted top move changes the crontab");
 
+# Saving a job needs a real Unix user, so check a valid save in a per-user
+# crontab, as on Linux. Plain shell commands stand in for the crontab
+# command, so the real one never runs.
+write_cron_config("cron_copy_command=cat >$crondir/USER",
+		  "cron_delete_command=rm -f $crondir/USER");
+$cronfile = "$crondir/root";
+write_crontab();
+$result = run_cgi("save_cron.cgi",
+	{ %edit, 'all_mins' => 0, 'mins' => [ 5, 6, 30 ],
+	  'range_def' => 0, 'range_start_day' => 1,
+	  'range_start_month' => 1, 'range_start_year' => 2026,
+	  'range_end_day' => 31, 'range_end_month' => 12,
+	  'range_end_year' => 2026 });
+like($result->{'out'}, qr/^Location:/m,
+	"save_cron.cgi accepts valid times and dates");
+like(read_crontab_fixture(),
+	qr/^5-6,30 \* \* \* \* \S+range\.pl 1-1-2026 31-12-2026 echo\\ first$/m,
+	"the saved job has the expected times and date range");
+
 done_testing();