CVE-2023-30607: Cross-Site Request Forgery (CSRF) (CWE-352)
value: icingaweb2-module-jira provides integration with Atlassian Jira. Starting in version 1.3.0 and prior to version 1.3.2, template and field configuration forms perform the deletion action before user input is validated, including the cross site request forgery token. This issue is fixed in version 1.3.2. There are no known workarounds.
CWE description: The web application does not, or can not, sufficiently verify whether a well-formed, valid, consistent request was intentionally provided by the user who submitted the request.
CVSS v3 base score: 8.8
Published: 2023-07-05T18:15Z
Repository: https://github.com/Icinga/icingaweb2-module-jira
Language: PHP
Files changed: application/forms/Config/FieldConfigForm.php, application/forms/Config/TemplateConfigForm.php
Commit date: 2023-07-03 16:23:07 +0200
Commit message: Do not perform deletion before user input is validated in `FieldConfigForm` and `TemplateConfigForm`

This fixes susceptibility to CSRF attacks.

Diff: diff --git a/application/forms/Config/FieldConfigForm.php b/application/forms/Config/FieldConfigForm.php
index 5b23b83..8b29ee6 100644
--- a/application/forms/Config/FieldConfigForm.php
+++ b/application/forms/Config/FieldConfigForm.php
@@ -29,13 +29,10 @@ class FieldConfigForm extends CompatForm
     protected $fieldValue;
 
     /** @var string */
     protected $templateName;
 
-    /** @var bool Hack used for delete button */
-    protected $callOnSuccess;
-
     /** @var string */
     protected $fieldId;
 
     public function __construct(RestApi $jira, string $templateName, $fieldId = null)
     {
@@ -45,12 +42,13 @@ class FieldConfigForm extends CompatForm
         $this->templateConfig = Config::module('jira', 'templates');
 
         $this->templateName = $templateName;
 
         if ($fieldId !== null) {
+            // obtain field key in case the fieldId is field label
             if (! array_key_exists($fieldId, $this->fields)) {
-                $this->fieldId = array_search($fieldId, $this->fields);
+                $this->fieldId = array_search($fieldId, $this->fields) ?: $fieldId;
             } else {
                 $this->fieldId = $fieldId;
             }
 
             $templateFields = $this->templateConfig->getSection($templateName)->toArray();
@@ -116,10 +114,11 @@ class FieldConfigForm extends CompatForm
                 'required'   => true,
                 'validators' => [
                     'Callback' => function ($value, $validator) {
                         /** @var CallbackValidator $validator */
                         $templateFieldKeys = $this->templateConfig->getSection($this->templateName)->keys();
+
                         $selected = $this->fields[$value];
 
                         if (
                             $value !== $this->fieldId
                             && (in_array($value, $templateFieldKeys) || in_array($selected, $templateFieldKeys))
@@ -335,27 +334,35 @@ class FieldConfigForm extends CompatForm
 
             $this->registerElement($deleteButton);
             $this->getElement('submit')
                 ->getWrapper()
                 ->prepend($deleteButton);
+        }
+    }
 
-            if ($deleteButton->hasBeenPressed()) {
-                $templateFields =  $this->templateConfig->getSection($this->templateName)->toArray();
-
-                $field = isset($templateFields[$this->fieldId]) ? $this->fieldId : $this->fields[$this->fieldId];
-
-                unset($templateFields[$field]);
+    public function hasBeenSubmitted()
+    {
+        if ($this->getPressedSubmitElement() !== null && $this->getPressedSubmitElement()->getName() === 'delete') {
+            return true;
+        }
 
-                $this->templateConfig->setSection($this->templateName, $templateFields);
-                $this->templateConfig->saveIni();
-                $this->getSubmitButton()->setValue($this->getSubmitButton()->getButtonLabel());
+        return parent::hasBeenSubmitted();
+    }
 
-                $this->callOnSuccess = false;
+    public function isValid()
+    {
+        if ($this->getPressedSubmitElement()->getName() === 'delete') {
+            $csrfElement = $this->getElement('CSRFToken');
 
-                return;
+            if (! $csrfElement->isValid()) {
+                return false;
             }
+
+            return true;
         }
+
+        return parent::isValid();
     }
 
     /**
      * Appends a null option to the given key-value pairs
      *
@@ -373,12 +380,19 @@ class FieldConfigForm extends CompatForm
         return [null => $nullLabel] + $enum;
     }
 
     public function onSuccess()
     {
-        if ($this->callOnSuccess === false) {
-            $this->getPressedSubmitElement()->setValue($this->getElement('delete')->getLabel());
+        if ($this->getPressedSubmitElement()->getName() === 'delete') {
+            $templateFields =  $this->templateConfig->getSection($this->templateName)->toArray();
+
+            $field = isset($templateFields[$this->fieldId]) ? $this->fieldId : $this->fields[$this->fieldId];
+
+            unset($templateFields[$field]);
+
+            $this->templateConfig->setSection($this->templateName, $templateFields);
+            $this->templateConfig->saveIni();
 
             return;
         }
 
         $fields = $this->templateConfig->getSection($this->templateName)->toArray();
diff --git a/application/forms/Config/TemplateConfigForm.php b/application/forms/Config/TemplateConfigForm.php
index cd2fa31..4d34698 100644
--- a/application/forms/Config/TemplateConfigForm.php
+++ b/application/forms/Config/TemplateConfigForm.php
@@ -19,13 +19,10 @@ class TemplateConfigForm extends CompatForm
     protected $config;
 
     /** @var string|null */
     protected $templateName;
 
-    /** @var bool Hack used for delete button */
-    protected $callOnSuccess;
-
     public function __construct($templateName = null)
     {
         $this->config = Config::module('jira', 'templates');
 
         $this->templateName = $templateName;
@@ -82,29 +79,46 @@ class TemplateConfigForm extends CompatForm
                     'formnovalidate' => true
                 ]
             );
 
             $this->registerElement($deleteButton);
-            $this->getElement('submit')->getWrapper()->prepend($deleteButton);
+            $this->getElement('submit')
+                ->getWrapper()
+                ->prepend($deleteButton);
+        }
+    }
 
-            if ($deleteButton->hasBeenPressed()) {
-                $this->config->removeSection($this->templateName);
-                $this->config->saveIni();
+    public function hasBeenSubmitted()
+    {
+        if ($this->getPressedSubmitElement() !== null && $this->getPressedSubmitElement()->getName() === 'delete') {
+            return true;
+        }
 
-                // Stupid cheat because ipl/html is not capable of multiple submit buttons
-                $this->getSubmitButton()->setValue($this->getSubmitButton()->getButtonLabel());
-                $this->callOnSuccess = false;
+        return parent::hasBeenSubmitted();
+    }
 
-                return;
+    public function isValid()
+    {
+        if ($this->getPressedSubmitElement()->getName() === 'delete') {
+            $csrfElement = $this->getElement('CSRFToken');
+
+            if (! $csrfElement->isValid()) {
+                return false;
             }
+
+            return true;
         }
+
+        return parent::isValid();
     }
 
     public function onSuccess()
     {
-        if ($this->callOnSuccess === false) {
-            $this->getSubmitButton()->setValue($this->getElement('delete')->getButtonLabel());
+        if ($this->getPressedSubmitElement()->getName() === 'delete') {
+            $this->config->removeSection($this->templateName);
+            $this->config->saveIni();
+
             return;
         }
 
         $templateConfig = Config::fromIni($this->config->getConfigFile());
         $value = $this->getValue('template');

Vulnerable code:     /** @var bool Hack used for delete button */
    protected $callOnSuccess;

                $this->fieldId = array_search($fieldId, $this->fields);
            if ($deleteButton->hasBeenPressed()) {
                $templateFields =  $this->templateConfig->getSection($this->templateName)->toArray();

                $field = isset($templateFields[$this->fieldId]) ? $this->fieldId : $this->fields[$this->fieldId];

                unset($templateFields[$field]);
                $this->templateConfig->setSection($this->templateName, $templateFields);
                $this->templateConfig->saveIni();
                $this->getSubmitButton()->setValue($this->getSubmitButton()->getButtonLabel());
                $this->callOnSuccess = false;
                return;
        if ($this->callOnSuccess === false) {
            $this->getPressedSubmitElement()->setValue($this->getElement('delete')->getLabel());
    /** @var bool Hack used for delete button */
    protected $callOnSuccess;

            $this->getElement('submit')->getWrapper()->prepend($deleteButton);
            if ($deleteButton->hasBeenPressed()) {
                $this->config->removeSection($this->templateName);
                $this->config->saveIni();
                // Stupid cheat because ipl/html is not capable of multiple submit buttons
                $this->getSubmitButton()->setValue($this->getSubmitButton()->getButtonLabel());
                $this->callOnSuccess = false;
                return;
        if ($this->callOnSuccess === false) {
            $this->getSubmitButton()->setValue($this->getElement('delete')->getButtonLabel());
Fixed code:             // obtain field key in case the fieldId is field label
                $this->fieldId = array_search($fieldId, $this->fields) ?: $fieldId;

        }
    }
    public function hasBeenSubmitted()
    {
        if ($this->getPressedSubmitElement() !== null && $this->getPressedSubmitElement()->getName() === 'delete') {
            return true;
        }
        return parent::hasBeenSubmitted();
    }
    public function isValid()
    {
        if ($this->getPressedSubmitElement()->getName() === 'delete') {
            $csrfElement = $this->getElement('CSRFToken');
            if (! $csrfElement->isValid()) {
                return false;

            return true;

        return parent::isValid();
        if ($this->getPressedSubmitElement()->getName() === 'delete') {
            $templateFields =  $this->templateConfig->getSection($this->templateName)->toArray();

            $field = isset($templateFields[$this->fieldId]) ? $this->fieldId : $this->fields[$this->fieldId];

            unset($templateFields[$field]);

            $this->templateConfig->setSection($this->templateName, $templateFields);
            $this->templateConfig->saveIni();
            $this->getElement('submit')
                ->getWrapper()
                ->prepend($deleteButton);
        }
    }
    public function hasBeenSubmitted()
    {
        if ($this->getPressedSubmitElement() !== null && $this->getPressedSubmitElement()->getName() === 'delete') {
            return true;
        }
        return parent::hasBeenSubmitted();
    }
    public function isValid()
    {
        if ($this->getPressedSubmitElement()->getName() === 'delete') {
            $csrfElement = $this->getElement('CSRFToken');

            if (! $csrfElement->isValid()) {
                return false;

            return true;

        return parent::isValid();
        if ($this->getPressedSubmitElement()->getName() === 'delete') {
            $this->config->removeSection($this->templateName);
            $this->config->saveIni();