CVE-2021-4033: Cross-Site Request Forgery (CSRF) (CWE-352)
value: kimai2 is vulnerable to Cross-Site Request Forgery (CSRF)
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: 6.5
Published: 2021-12-09T20:15Z
Repository: https://github.com/kevinpapst/kimai2
Language: PHP
Files changed: src/Controller/AbstractController.php, src/Controller/InvoiceController.php, src/Form/Extension/SelectWithApiDataExtension.php
Commit date: 2021-12-04 01:18:49 +0100
Commit message: fix invoice create and search (#2990)


Diff: diff --git a/src/Controller/AbstractController.php b/src/Controller/AbstractController.php
index 9fd50f654..109857b8b 100644
--- a/src/Controller/AbstractController.php
+++ b/src/Controller/AbstractController.php
@@ -216,10 +216,12 @@ abstract class AbstractController extends BaseAbstractController implements Serv
                     return false;
                 }
             }
         }
 
+        $request->query->remove('_token');
+
         if ($request->query->has('resetSearchFilter')) {
             $data->resetFilter();
             $this->removeLastSearch($data);
 
             return true;
diff --git a/src/Controller/InvoiceController.php b/src/Controller/InvoiceController.php
index 790ecc7e1..d1a209d40 100644
--- a/src/Controller/InvoiceController.php
+++ b/src/Controller/InvoiceController.php
@@ -64,11 +64,11 @@ final class InvoiceController extends AbstractController
 
     /**
      * @Route(path="/", name="invoice", methods={"GET", "POST"})
      * @Security("is_granted('view_invoice')")
      */
-    public function indexAction(Request $request, SystemConfiguration $configuration): Response
+    public function indexAction(Request $request, SystemConfiguration $configuration, CsrfTokenManagerInterface $csrfTokenManager): Response
     {
         if (!$this->templateRepository->hasTemplate()) {
             if ($this->isGranted('manage_invoice_template')) {
                 return $this->redirectToRoute('admin_invoice_template_create');
             }
@@ -98,10 +98,12 @@ final class InvoiceController extends AbstractController
                     $this->flashError('action.csrf.error');
 
                     return $this->redirectToRoute('invoice');
                 }
 
+                $csrfTokenManager->refreshToken('invoice.create');
+
                 try {
                     return $this->renderInvoice($query, $request);
                 } catch (Exception $ex) {
                     $this->logException($ex);
                     $this->flashError('action.update.error', ['%reason%' => 'check doctor/logs']);
@@ -146,10 +148,11 @@ final class InvoiceController extends AbstractController
         $form = $this->getToolbarForm($query, $configuration->find('invoice.simple_form'));
         $form->submit($request->query->all(), false);
 
         if ($form->isValid()) {
             try {
+                $query->setCustomers([$customer]);
                 $model = $this->service->createModel($query);
 
                 return $this->service->renderInvoiceWithModel($model, $this->dispatcher);
             } catch (Exception $ex) {
                 $this->logException($ex);
@@ -165,11 +168,11 @@ final class InvoiceController extends AbstractController
     /**
      * @Route(path="/save-invoice/{customer}/{template}", name="invoice_create", methods={"GET"})
      * @Security("is_granted('access', customer)")
      * @Security("is_granted('create_invoice')")
      */
-    public function createInvoiceAction(Customer $customer, InvoiceTemplate $template, Request $request, SystemConfiguration $configuration): Response
+    public function createInvoiceAction(Customer $customer, InvoiceTemplate $template, Request $request, SystemConfiguration $configuration, CsrfTokenManagerInterface $csrfTokenManager): Response
     {
         if (!$this->templateRepository->hasTemplate()) {
             return $this->redirectToRoute('invoice');
         }
 
@@ -183,13 +186,17 @@ final class InvoiceController extends AbstractController
             $this->flashError('action.csrf.error');
 
             return $this->redirectToRoute('invoice');
         }
 
+        $csrfTokenManager->refreshToken('invoice.create');
+
         $query = $this->getDefaultQuery();
         $form = $this->getToolbarForm($query, $configuration->find('invoice.simple_form'));
-        $form->submit($request->query->all(), false);
+        if ($this->handleSearch($form, $request)) {
+            return $this->redirectToRoute('invoice');
+        }
 
         if ($form->isValid()) {
             $query->setTemplate($template);
             $query->setCustomers([$customer]);
 
diff --git a/src/Form/Extension/SelectWithApiDataExtension.php b/src/Form/Extension/SelectWithApiDataExtension.php
index 53fe8a614..7dd901a53 100644
--- a/src/Form/Extension/SelectWithApiDataExtension.php
+++ b/src/Form/Extension/SelectWithApiDataExtension.php
@@ -73,12 +73,16 @@ class SelectWithApiDataExtension extends AbstractTypeExtension
         do {
             $formPrefixes[] = $parent->getName();
         } while (($parent = $parent->getParent()) !== null);
 
         $formPrefix = implode('_', array_reverse($formPrefixes));
+        $formField = $apiData['select'];
 
-        $formField = $formPrefix . '_' . $apiData['select'];
+        // forms with prefix (like toolbar & search) would result in a wrong field name "_foo" instead of "foo"
+        if ($formPrefix !== '') {
+            $formField = $formPrefix . '_' . $apiData['select'];
+        }
 
         $view->vars['attr'] = array_merge($view->vars['attr'], [
             'data-form-prefix' => $formPrefix,
             'data-related-select' => $formField,
             'data-api-url' => $this->router->generate($apiData['route'], $apiData['route_params']),

Vulnerable code:     public function indexAction(Request $request, SystemConfiguration $configuration): Response
    public function createInvoiceAction(Customer $customer, InvoiceTemplate $template, Request $request, SystemConfiguration $configuration): Response
        $form->submit($request->query->all(), false);
        $formField = $formPrefix . '_' . $apiData['select'];
Fixed code:         $request->query->remove('_token');

    public function indexAction(Request $request, SystemConfiguration $configuration, CsrfTokenManagerInterface $csrfTokenManager): Response
                $csrfTokenManager->refreshToken('invoice.create');

                $query->setCustomers([$customer]);
    public function createInvoiceAction(Customer $customer, InvoiceTemplate $template, Request $request, SystemConfiguration $configuration, CsrfTokenManagerInterface $csrfTokenManager): Response
        $csrfTokenManager->refreshToken('invoice.create');

        if ($this->handleSearch($form, $request)) {
            return $this->redirectToRoute('invoice');
        }
        $formField = $apiData['select'];
        // forms with prefix (like toolbar & search) would result in a wrong field name "_foo" instead of "foo"
        if ($formPrefix !== '') {
            $formField = $formPrefix . '_' . $apiData['select'];
        }