CVE-2023-5498: Cross-Site Request Forgery (CSRF) (CWE-352)
value: Cross-Site Request Forgery (CSRF) in GitHub repository chiefonboarding/chiefonboarding prior to v2.0.47.
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: 4.3
Published: 2023-10-10T10:15Z
Repository: https://github.com/chiefonboarding/chiefonboarding
Language: Python
Files changed: back/admin/admin_tasks/templates/admin_tasks_detail.html, back/admin/admin_tasks/tests.py, back/admin/admin_tasks/views.py
Commit date: 2023-10-02 23:31:06 +0200
Commit message: Fix missing csrf token on admin task done view (#367)


Diff: diff --git a/back/admin/admin_tasks/templates/admin_tasks_detail.html b/back/admin/admin_tasks/templates/admin_tasks_detail.html
index 42d70cb..24d5963 100644
--- a/back/admin/admin_tasks/templates/admin_tasks_detail.html
+++ b/back/admin/admin_tasks/templates/admin_tasks_detail.html
@@ -1,16 +1,19 @@
 {% extends 'admin_base.html' %}
 {% load i18n %}
 {% load crispy_forms_tags %}
 {% block actions %}
-<a type="submit" href="{% url 'admin_tasks:completed' object.id %}" class="btn btn-primary">
-  {% if object.completed %}
-    {% translate "Reopen" %}
-  {% else %}
-    {% translate "Complete" %}
-  {% endif %}
-</a>
+<form method="post" action="{% url 'admin_tasks:completed' object.id %}">
+  {% csrf_token %}
+  <button type="submit" class="btn btn-primary">
+    {% if object.completed %}
+      {% translate "Reopen" %}
+    {% else %}
+      {% translate "Complete" %}
+    {% endif %}
+  </button>
+</form>
 {% endblock %}
 
 {% block content %}
 <div class="row">
   <div class="col-4">
diff --git a/back/admin/admin_tasks/tests.py b/back/admin/admin_tasks/tests.py
index a50b5a0..b6911e8 100644
--- a/back/admin/admin_tasks/tests.py
+++ b/back/admin/admin_tasks/tests.py
@@ -290,11 +290,11 @@ def test_complete_admin_task(client, admin_factory, admin_task_factory):
 
     complete_url = reverse("admin_tasks:completed", args=[task1.id])
     assert "Complete" in response.content.decode()
     assert complete_url in response.content.decode()
 
-    response = client.get(complete_url, follow=True)
+    response = client.post(complete_url, follow=True)
     task1.refresh_from_db()
     task2.refresh_from_db()
 
     assert "Complete" not in response.content.decode()
     assert "completed" in response.content.decode()
diff --git a/back/admin/admin_tasks/views.py b/back/admin/admin_tasks/views.py
index 6075b66..0d2111d 100644
--- a/back/admin/admin_tasks/views.py
+++ b/back/admin/admin_tasks/views.py
@@ -1,11 +1,12 @@
 from django.contrib.messages.views import SuccessMessageMixin
 from django.http import Http404
-from django.shortcuts import get_object_or_404
+from django.shortcuts import get_object_or_404, redirect
 from django.urls import reverse, reverse_lazy
 from django.utils.translation import gettext as _
-from django.views.generic.base import RedirectView
+
+from django.views.generic.detail import BaseDetailView
 from django.views.generic.edit import CreateView, UpdateView
 from django.views.generic.list import ListView
 
 from users.mixins import LoginRequiredMixin, ManagerPermMixin
 
@@ -43,20 +44,18 @@ class AllAdminTasksListView(LoginRequiredMixin, ManagerPermMixin, ListView):
         context["subtitle"] = "Tasks"
         context["add_action"] = reverse_lazy("admin_tasks:create")
         return context
 
 
-class AdminTaskToggleDoneView(LoginRequiredMixin, ManagerPermMixin, RedirectView):
-    permanent = False
-    pattern_name = "admin_tasks:detail"
+class AdminTaskToggleDoneView(LoginRequiredMixin, ManagerPermMixin, BaseDetailView):
+    model = AdminTask
 
-    def get(self, request, *args, **kwargs):
-        task_id = self.kwargs.get("pk", -1)
-        admin_task = get_object_or_404(AdminTask, id=task_id)
+    def post(self, request, *args, **kwargs):
+        admin_task = self.get_object()
         admin_task.completed = not admin_task.completed
         admin_task.save()
-        return super().get(request, *args, **kwargs)
+        return redirect("admin_tasks:detail", pk=admin_task.id)
 
 
 class AdminTasksCreateView(
     LoginRequiredMixin, ManagerPermMixin, SuccessMessageMixin, CreateView
 ):

Vulnerable code: <a type="submit" href="{% url 'admin_tasks:completed' object.id %}" class="btn btn-primary">
  {% if object.completed %}
    {% translate "Reopen" %}
  {% else %}
    {% translate "Complete" %}
  {% endif %}
</a>
    response = client.get(complete_url, follow=True)
from django.shortcuts import get_object_or_404
from django.views.generic.base import RedirectView
class AdminTaskToggleDoneView(LoginRequiredMixin, ManagerPermMixin, RedirectView):
    permanent = False
    pattern_name = "admin_tasks:detail"
    def get(self, request, *args, **kwargs):
        task_id = self.kwargs.get("pk", -1)
        admin_task = get_object_or_404(AdminTask, id=task_id)
        return super().get(request, *args, **kwargs)
Fixed code: <form method="post" action="{% url 'admin_tasks:completed' object.id %}">
  {% csrf_token %}
  <button type="submit" class="btn btn-primary">
    {% if object.completed %}
      {% translate "Reopen" %}
    {% else %}
      {% translate "Complete" %}
    {% endif %}
  </button>
</form>
    response = client.post(complete_url, follow=True)
from django.shortcuts import get_object_or_404, redirect

from django.views.generic.detail import BaseDetailView
class AdminTaskToggleDoneView(LoginRequiredMixin, ManagerPermMixin, BaseDetailView):
    model = AdminTask
    def post(self, request, *args, **kwargs):
        admin_task = self.get_object()
        return redirect("admin_tasks:detail", pk=admin_task.id)