From a8c348cfa437b918228a5a3ac0cb4e8eb538047e Mon Sep 17 00:00:00 2001 From: Franco Fichtner Date: Mon, 11 Sep 2023 11:40:00 +0200 Subject: [PATCH] system: cron parameters are escaped properly nowadays This is allowed nowadays with the proper escaping in the template employed. However... 1. The parameter"s" are enforced by doing white-space separated passing of individiual parts, but that breaks backend scripts expecting either spaces to be part of the parmeter or discarding additional parameters. This matters, because... 2. https://docs.opnsense.org/manual/settingsmenu.html#cron does not state any two parameter value of interest to users apart from custom Cron glue. I'd rather have "parameters" treated as a single first parameter which can be passed with a %s to the shell, but I'm unsure if configd will treat it that way? At least the crontab part would not be the issue. Let's test this theory: # cat src/opnsense/service/conf/actions.d/actions_test.conf [shell] command:/bin/csh -c parameters:%s message:Running %s type:script_output description:Shell execution (use with care) # configctl test shell "echo foo" Parameter mismatch # configctl test shell "echo\ foo" foo # configctl test shell "echo\ foo;echo\ bar" foo bar So there seems to be a mishandling of spaces in general which is probably why the parameters are treated as such in the crontab file. Perhaps we need to discuss this. --- src/opnsense/mvc/app/models/OPNsense/Cron/Cron.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/opnsense/mvc/app/models/OPNsense/Cron/Cron.xml b/src/opnsense/mvc/app/models/OPNsense/Cron/Cron.xml index b2a2d9c18..d65578daa 100644 --- a/src/opnsense/mvc/app/models/OPNsense/Cron/Cron.xml +++ b/src/opnsense/mvc/app/models/OPNsense/Cron/Cron.xml @@ -57,8 +57,8 @@ Y - /^([^;|`]){1,255}$/ - Enter valid parameter(s) for the chosen command (Found illegal characters). + /^(.){1,255}$/ + Input too long. /^([\t\n\v\f\r 0-9a-zA-Z.,_\x{00A0}-\x{FFFF}]){1,255}$/u