From 6a79436af360d71140771d27f07ff1ad1bb45460 Mon Sep 17 00:00:00 2001 From: Markus Frosch Date: Tue, 26 Mar 2019 12:02:13 +0100 Subject: [PATCH 1/4] IcingaCommand: Allow command to be rendered as string Icinga 2 allows this from the beginning. --- application/forms/IcingaCommandForm.php | 15 +++++++ library/Director/Objects/IcingaCommand.php | 48 ++++++++++++++++------ schema/mysql-migrations/upgrade_160.sql | 6 +++ schema/mysql.sql | 3 +- schema/pgsql-migrations/upgrade_160.sql | 6 +++ schema/pgsql.sql | 3 +- 6 files changed, 66 insertions(+), 15 deletions(-) create mode 100644 schema/mysql-migrations/upgrade_160.sql create mode 100644 schema/pgsql-migrations/upgrade_160.sql diff --git a/application/forms/IcingaCommandForm.php b/application/forms/IcingaCommandForm.php index eacc747d..bba4278a 100644 --- a/application/forms/IcingaCommandForm.php +++ b/application/forms/IcingaCommandForm.php @@ -73,6 +73,21 @@ class IcingaCommandForm extends DirectorObjectForm . ' specific unit (e.g. 1m or also 3m 30s).' ) )); + + $descIsString = [ + $this->translate('Render the command as a plain string instead of an array.'), + $this->translate('This can not be used together with arguments.'), + $this->translate('It is disabled by default and should only be used in rare cases.'), + ]; + + $this->addBoolean( + 'is_string', + array( + 'label' => $this->translate('Render as string'), + 'description' => join(' ', $descIsString), + ) + ); + $this->addDisabledElement(); $this->addZoneSection(); $this->setButtons(); diff --git a/library/Director/Objects/IcingaCommand.php b/library/Director/Objects/IcingaCommand.php index 4486cb8d..e77b0e8b 100644 --- a/library/Director/Objects/IcingaCommand.php +++ b/library/Director/Objects/IcingaCommand.php @@ -19,14 +19,19 @@ class IcingaCommand extends IcingaObject implements ObjectWithArguments, ExportI protected $type = 'CheckCommand'; protected $defaultProperties = [ - 'id' => null, - 'object_name' => null, - 'object_type' => null, - 'disabled' => 'n', - 'methods_execute' => null, - 'command' => null, - 'timeout' => null, - 'zone_id' => null, + 'id' => null, + 'object_name' => null, + 'object_type' => null, + 'disabled' => 'n', + 'methods_execute' => null, + 'command' => null, + 'timeout' => null, + 'zone_id' => null, + 'is_string' => null, + ]; + + protected $booleans = [ + 'is_string' => 'is_string', ]; protected $supportsCustomVars = true; @@ -288,16 +293,33 @@ class IcingaCommand extends IcingaObject implements ObjectWithArguments, ExportI $command = $this->get('command'); $prefix = ''; if (preg_match('~^([A-Z][A-Za-z0-9_]+\s\+\s)(.+?)$~', $command, $m)) { - $prefix = $m[1]; + $prefix = $m[1]; $command = $m[2]; } elseif (! $this->isAbsolutePath($command)) { $prefix = 'PluginDir + '; $command = '/' . $command; } - $parts = preg_split('/\s+/', $command, -1, PREG_SPLIT_NO_EMPTY); - array_unshift($parts, c::alreadyRendered($prefix . c::renderString(array_shift($parts)))); - - return c::renderKeyValue('command', c::renderArray($parts)); + + $inherited = $this->getInheritedProperties(); + + if ($this->get('is_string') === 'y' || ($this->get('is_string') === null + && property_exists($inherited, 'is_string') && $inherited->is_string === 'y')) { + return c::renderKeyValue('command', $prefix . c::renderString($command)); + } else { + $parts = preg_split('/\s+/', $command, -1, PREG_SPLIT_NO_EMPTY); + array_unshift($parts, c::alreadyRendered($prefix . c::renderString(array_shift($parts)))); + + return c::renderKeyValue('command', c::renderArray($parts)); + } + } + + /** + * @codingStandardsIgnoreStart + */ + protected function renderIs_string() + { + // @codingStandardsIgnoreEnd + return ''; } protected function isAbsolutePath($path) diff --git a/schema/mysql-migrations/upgrade_160.sql b/schema/mysql-migrations/upgrade_160.sql new file mode 100644 index 00000000..3afbf47a --- /dev/null +++ b/schema/mysql-migrations/upgrade_160.sql @@ -0,0 +1,6 @@ +ALTER TABLE icinga_command + ADD COLUMN is_string enum ('y', 'n') NULL; + +INSERT INTO director_schema_migration + (schema_version, migration_time) + VALUES (160, NOW()); diff --git a/schema/mysql.sql b/schema/mysql.sql index bbb5205f..4b6f7e0b 100644 --- a/schema/mysql.sql +++ b/schema/mysql.sql @@ -320,6 +320,7 @@ CREATE TABLE icinga_command ( disabled ENUM('y', 'n') NOT NULL DEFAULT 'n', methods_execute VARCHAR(64) DEFAULT NULL, command TEXT DEFAULT NULL, + is_string ENUM('y', 'n') NULL, -- env text DEFAULT NULL, -- vars text DEFAULT NULL, timeout SMALLINT UNSIGNED DEFAULT NULL, @@ -1787,4 +1788,4 @@ CREATE TABLE icinga_timeperiod_exclude ( INSERT INTO director_schema_migration (schema_version, migration_time) - VALUES (159, NOW()); + VALUES (160, NOW()); diff --git a/schema/pgsql-migrations/upgrade_160.sql b/schema/pgsql-migrations/upgrade_160.sql new file mode 100644 index 00000000..5258146b --- /dev/null +++ b/schema/pgsql-migrations/upgrade_160.sql @@ -0,0 +1,6 @@ +ALTER TABLE icinga_command + ADD COLUMN is_string enum_boolean NULL; + +INSERT INTO director_schema_migration + (schema_version, migration_time) + VALUES (160, NOW()); diff --git a/schema/pgsql.sql b/schema/pgsql.sql index 6ec87cc9..09cb381e 100644 --- a/schema/pgsql.sql +++ b/schema/pgsql.sql @@ -415,6 +415,7 @@ CREATE TABLE icinga_command ( disabled enum_boolean NOT NULL DEFAULT 'n', methods_execute character varying(64) DEFAULT NULL, command text DEFAULT NULL, + is_string enum_boolean NULL, -- env text DEFAULT NULL, timeout smallint DEFAULT NULL, zone_id integer DEFAULT NULL, @@ -2086,4 +2087,4 @@ CREATE TABLE icinga_timeperiod_exclude ( INSERT INTO director_schema_migration (schema_version, migration_time) - VALUES (158, NOW()); + VALUES (160, NOW()); From e64ace7ccf4ca63a0acb00f906eef4e43c978033 Mon Sep 17 00:00:00 2001 From: Markus Frosch Date: Tue, 26 Mar 2019 11:38:04 +0100 Subject: [PATCH 2/4] IcingaCommandForm: Add warning for command as plain string --- application/forms/IcingaCommandForm.php | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/application/forms/IcingaCommandForm.php b/application/forms/IcingaCommandForm.php index bba4278a..0524f574 100644 --- a/application/forms/IcingaCommandForm.php +++ b/application/forms/IcingaCommandForm.php @@ -76,8 +76,9 @@ class IcingaCommandForm extends DirectorObjectForm $descIsString = [ $this->translate('Render the command as a plain string instead of an array.'), - $this->translate('This can not be used together with arguments.'), - $this->translate('It is disabled by default and should only be used in rare cases.'), + $this->translate('If enabled you can not define arguments.'), + $this->translate('Disabled by default, and should only be used in rare cases.'), + $this->translate('WARNING, this can allow shell script injection via custom variables used in command.'), ]; $this->addBoolean( From a6e32d763f2080e99f36d0174ec78c2f85a3da8d Mon Sep 17 00:00:00 2001 From: Markus Frosch Date: Tue, 26 Mar 2019 11:58:06 +0100 Subject: [PATCH 3/4] ObjectPreview: Avoid linking a flat command attribute inside a command --- library/Director/Web/ObjectPreview.php | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/library/Director/Web/ObjectPreview.php b/library/Director/Web/ObjectPreview.php index 96eda238..f54009a2 100644 --- a/library/Director/Web/ObjectPreview.php +++ b/library/Director/Web/ObjectPreview.php @@ -99,17 +99,23 @@ class ObjectPreview $classes[] = 'logfile'; } + $type = $object->getShortTableName(); + $plain = Html::wantHtml($file->getContent())->render(); $plain = preg_replace_callback( '/^(\s+import\s+\"\;)(.+)(\"\;)/m', [$this, 'linkImport'], $plain ); - $plain = preg_replace_callback( - '/^(\s+(?:check_|event_)?command\s+=\s+\"\;)(.+)(\"\;)/m', - [$this, 'linkCommand'], - $plain - ); + + if ($type !== 'command') { + $plain = preg_replace_callback( + '/^(\s+(?:check_|event_)?command\s+=\s+\"\;)(.+)(\"\;)/m', + [$this, 'linkCommand'], + $plain + ); + } + $plain = preg_replace_callback( '/^(\s+host_name\s+=\s+\"\;)(.+)(\"\;)/m', [$this, 'linkHost'], From 2be27ddf3d844469af9e937b5f8174c5fada6b18 Mon Sep 17 00:00:00 2001 From: Markus Frosch Date: Tue, 26 Mar 2019 12:13:53 +0100 Subject: [PATCH 4/4] IcingaObjectInspection: Show flat commands run by the core --- .../Director/Web/Widget/IcingaObjectInspection.php | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/library/Director/Web/Widget/IcingaObjectInspection.php b/library/Director/Web/Widget/IcingaObjectInspection.php index fe16dbd0..0832d043 100644 --- a/library/Director/Web/Widget/IcingaObjectInspection.php +++ b/library/Director/Web/Widget/IcingaObjectInspection.php @@ -57,18 +57,21 @@ class IcingaObjectInspection extends BaseHtmlElement { $this->add(Html::tag('h2', null, $this->translate('Last Check Result'))); $this->renderCheckResultDetails($result); - if (property_exists($result, 'command') && is_array($result->command)) { + if (property_exists($result, 'command')) { $this->renderExecutedCommand($result->command); } } /** - * @param array $command + * @param array|string $command + * * @throws \Icinga\Exception\IcingaException */ - protected function renderExecutedCommand(array $command) + protected function renderExecutedCommand($command) { - $command = implode(' ', array_map('escapeshellarg', $command)); + if (is_array($command)) { + $command = implode(' ', array_map('escapeshellarg', $command)); + } $this->add([ Html::tag('h3', null, 'Executed Command'), $this->formatConsole($command)