Add CSRF protection for GET action endpoints

97282105 · Robin Sonnabend · fdf1600d · 97282105 · 97282105 · 97282105
Commit 97282105 authored 7 years ago by Robin Sonnabend
--- a/decorators.py
+++ b/decorators.py
-from flask import flash
+from flask import request, flash, abort
 from functools import wraps
 from models.database import ALL_MODELS
 from shared import current_user
+from utils import get_csrf_token
 import back
 ID_KEY = "id"
@@ -90,3 +91,14 @@ def require_publish_right(require_exist=True):
 def require_admin_right(require_exist=True):
    return require_right("admin", require_exist)
+def protect_csrf(function):
+    @wraps(function)
+    def _decorated_function(*args, **kwargs):
+        token = request.args.get("csrf_token")
+        if token != get_csrf_token():
+            print(token, get_csrf_token())
+            abort(400)
+        return function(*args, **kwargs)
+    return _decorated_function
--- a/server.py
+++ b/server.py
@@ -31,9 +31,9 @@ from shared import (
 from utils import (
    get_first_unused_int, get_etherpad_text, split_terms, optional_int_arg,
    fancy_join, footnote_hash, get_git_revision, get_max_page_length_exp,
-    get_internal_filename)
+    get_internal_filename, get_csrf_token)
 from decorators import (
-    db_lookup,
+    db_lookup, protect_csrf,
    require_private_view_right, require_modify_right, require_publish_right,
    require_admin_right)
 from models.database import (
@@ -89,6 +89,7 @@ app.jinja_env.filters["fancy_join"] = fancy_join
 app.jinja_env.filters["footnote_hash"] = footnote_hash
 app.jinja_env.tests["auth_valid"] = security_manager.check_user
 app.jinja_env.tests["needs_date"] = needs_date_test
+app.jinja_env.globals["get_csrf_token"] = get_csrf_token
 additional_templates = getattr(config, "LATEX_LOCAL_TEMPLATES", None)
 if additional_templates is not None and os.path.isdir(additional_templates):
@@ -336,6 +337,7 @@ def show_type(protocoltype):
 @app.route("/type/delete/<int:protocoltype_id>")
 @login_required
+@protect_csrf
 @db_lookup(ProtocolType)
 @require_admin_right()
 @require_modify_right()
@@ -382,6 +384,7 @@ def edit_reminder(meetingreminder):
 @app.route("/type/reminder/delete/<int:meetingreminder_id>")
 @login_required
+@protect_csrf
 @db_lookup(MeetingReminder)
 @require_modify_right()
 def delete_reminder(meetingreminder):
@@ -434,6 +437,7 @@ def edit_default_top(protocoltype, defaulttop):
 @app.route("/type/tops/delete/<int:defaulttop_id>")
 @login_required
+@protect_csrf
 @db_lookup(DefaultTOP)
 @require_modify_right()
 def delete_default_top(defaulttop):
@@ -445,6 +449,7 @@ def delete_default_top(defaulttop):
 @app.route("/type/tops/move/<int:defaulttop_id>/<diff>/")
 @login_required
+@protect_csrf
 @db_lookup(DefaultTOP)
 @require_modify_right()
 def move_default_top(defaulttop, diff):
@@ -649,6 +654,7 @@ def show_protocol(protocol):
 @app.route("/protocol/delete/<int:protocol_id>")
 @login_required
+@protect_csrf
 @db_lookup(Protocol)
 @require_admin_right()
 @require_modify_right()
@@ -663,6 +669,7 @@ def delete_protocol(protocol):
 @app.route("/protocol/etherpull/<int:protocol_id>")
 @login_required
+@protect_csrf
 @db_lookup(Protocol)
 @require_modify_right()
 def etherpull_protocol(protocol):
@@ -781,6 +788,7 @@ def upload_new_protocol_by_file():
 @app.route("/protocol/recompile/<int:protocol_id>")
 @login_required
+@protect_csrf
 @db_lookup(Protocol)
 @require_admin_right()
 @require_modify_right()
@@ -814,6 +822,7 @@ def get_protocol_template(protocol):
 @app.route("/protocol/etherpush/<int:protocol_id>")
 @login_required
+@protect_csrf
 @db_lookup(Protocol)
 @require_modify_right()
 def etherpush_protocol(protocol):
@@ -848,6 +857,7 @@ def update_protocol(protocol):
 @app.route("/protocol/publish/<int:protocol_id>")
 @login_required
+@protect_csrf
 @db_lookup(Protocol)
 @require_publish_right()
 def publish_protocol(protocol):
@@ -858,6 +868,7 @@ def publish_protocol(protocol):
 @app.route("/prococol/send/private/<int:protocol_id>")
 @login_required
+@protect_csrf
 @db_lookup(Protocol)
 @require_modify_right()
 def send_protocol_private(protocol):
@@ -871,6 +882,7 @@ def send_protocol_private(protocol):
 @app.route("/prococol/send/public/<int:protocol_id>")
 @login_required
+@protect_csrf
 @db_lookup(Protocol)
 @require_publish_right()
 def send_protocol_public(protocol):
@@ -884,6 +896,7 @@ def send_protocol_public(protocol):
 @app.route("/protocol/reminder/<int:protocol_id>")
 @login_required
+@protect_csrf
 @db_lookup(Protocol)
 @require_modify_right()
 def send_protocol_reminder(protocol):
@@ -947,6 +960,7 @@ def edit_top(top):
 @app.route("/protocol/top/delete/<int:top_id>")
 @login_required
+@protect_csrf
 @db_lookup(TOP)
 @require_modify_right()
 def delete_top(top):
@@ -961,6 +975,7 @@ def delete_top(top):
 @app.route("/protocol/top/move/<int:top_id>/<diff>")
 @login_required
+@protect_csrf
 @db_lookup(TOP)
 @require_modify_right()
 def move_top(top, diff):
@@ -1145,6 +1160,7 @@ def show_todo(todo):
 @app.route("/todo/delete/<int:todo_id>")
 @login_required
+@protect_csrf
 @db_lookup(Todo)
 @require_private_view_right()
 def delete_todo(todo):
@@ -1311,6 +1327,7 @@ def edit_document(document):
 @app.route("/document/delete/<int:document_id>")
 @login_required
+@protect_csrf
 @db_lookup(Document)
 @require_admin_right()
 @require_modify_right()
@@ -1325,6 +1342,7 @@ def delete_document(document):
 @app.route("/document/print/<int:document_id>")
 @login_required
+@protect_csrf
 @db_lookup(Document)
 @require_modify_right()
 def print_document(document):
@@ -1339,6 +1357,7 @@ def print_document(document):
 @app.route("/decision/print/<int:decisiondocument_id>")
 @login_required
+@protect_csrf
 @db_lookup(DecisionDocument)
 @require_modify_right()
 def print_decision(decisiondocument):
@@ -1383,6 +1402,7 @@ def show_error(error):
 @app.route("/error/delete/<int:error_id>")
 @login_required
+@protect_csrf
 @db_lookup(Error)
 @require_modify_right()
 def delete_error(error):
@@ -1434,6 +1454,7 @@ def edit_todomail(todomail):
 @app.route("/todomail/delete/<int:todomail_id>")
 @login_required
+@protect_csrf
 @db_lookup(TodoMail)
 def delete_todomail(todomail):
    name = todomail.name
@@ -1478,6 +1499,7 @@ def edit_defaultmeta(defaultmeta):
 @app.route("/defaultmeta/delete/<int:defaultmeta_id>")
 @login_required
+@protect_csrf
 @db_lookup(DefaultMeta)
 @require_admin_right()
 @require_modify_right()
@@ -1527,6 +1549,7 @@ def edit_decisioncategory(decisioncategory):
 @app.route("/decisioncategory/delete/<int:decisioncategory_id>")
 @login_required
+@protect_csrf
 @db_lookup(DecisionCategory)
 @require_admin_right()
 @require_modify_right()
@@ -1688,6 +1711,7 @@ def feed_appointments_ical(protocoltype):
 @app.route("/like/new")
 @login_required
+@protect_csrf
 def new_like():
    user = current_user()
    parent = None
@@ -1736,6 +1760,7 @@ def login():
 @app.route("/logout")
 @login_required
+@protect_csrf
 def logout():
    if "auth" in session:
        session.pop("auth")

--- a/templates/layout.html
+++ b/templates/layout.html
@@ -50,7 +50,7 @@
            </ul>
            <ul class="nav navbar-nav navbar-right">
                {% if check_login() %}
-                <li><a href="{{url_for("logout")}}">Logout</a></li>
+                <li><a href="{{url_for("logout", csrf_token=get_csrf_token())}}">Logout</a></li>
                {% else %}
                <li><a href="{{url_for("login")}}">Login</a></li>
                {% endif %}

--- a/templates/macros.html
+++ b/templates/macros.html
@@ -175,7 +175,7 @@ to not render a label for the CRSFTokenField -->
            {% set verb = "likes" %}
        {% endif %}
        {% if add_link %}
-        <a href="{{url_for("new_like", next=request.url, **kwargs)}}">
+        <a href="{{url_for("new_like", csrf_token=get_csrf_token(), **kwargs)}}">
        {% endif %}
        <div class="likes-div">
            <p>{{likes|length}} <span class="like-sign">&#x1f44d;</span></p>

--- a/templates/protocol-show.html
+++ b/templates/protocol-show.html
@@ -16,7 +16,7 @@
        <div class="btn-group">
            {% if has_modify_right %}
                {% if config.ETHERPAD_ACTIVE and not protocol.public %}
-                <a class="btn {% if protocol.source is none %}btn-primary{% else %}btn-default{% endif %}" href="{{url_for("etherpull_protocol", protocol_id=protocol.id)}}">Aus Etherpad</a>
+                <a class="btn {% if protocol.source is none %}btn-primary{% else %}btn-default{% endif %}" href="{{url_for("etherpull_protocol", protocol_id=protocol.id, csrf_token=get_csrf_token())}}">Aus Etherpad</a>
                {% endif %}
                {% if protocol.source is not none %}
                    <a class="btn btn-primary" href="{{url_for("get_protocol_source", protocol_id=protocol.id)}}">Quelltext</a>
@@ -26,23 +26,23 @@
                {% endif %}
                {% if not protocol.public %}
                    {% if config.ETHERPAD_ACTIVE %}
-                    <a class="btn btn-primary" href="{{url_for("etherpush_protocol", protocol_id=protocol.id)}}"{% if large_time_diff %} onclick="return confirm('Bist du dir sicher, dass du das Template bereits in das Etherpad kopieren willst? Die Sitzung ist erst {% if time_diff.days != 1 %}in {{time_diff.days}} Tagen{% else %}morgen{% endif %}.');"{% endif %} target="_blank">Etherpad</a>
+                    <a class="btn btn-primary" href="{{url_for("etherpush_protocol", protocol_id=protocol.id, csrf_token=get_csrf_token())}}"{% if large_time_diff %} onclick="return confirm('Bist du dir sicher, dass du das Template bereits in das Etherpad kopieren willst? Die Sitzung ist erst {% if time_diff.days != 1 %}in {{time_diff.days}} Tagen{% else %}morgen{% endif %}.');"{% endif %} target="_blank">Etherpad</a>
                    {% endif %}
                {% endif %}
                {% if not protocol.is_done() %}
                    <a class="btn btn-default" href="{{url_for("get_protocol_template", protocol_id=protocol.id)}}">Vorlage</a>
                    {% if config.MAIL_ACTIVE %}
-                        <a class="btn btn-default" href="{{url_for("send_protocol_reminder", protocol_id=protocol.id)}}" onclick="return confirm('Bist du dir sicher, dass du manuell eine Einladung verschicken willst? Dies wird auch automatisch geschehen.');">Einladung versenden</a>
+                        <a class="btn btn-default" href="{{url_for("send_protocol_reminder", protocol_id=protocol.id, csrf_token=get_csrf_token())}}" onclick="return confirm('Bist du dir sicher, dass du manuell eine Einladung verschicken willst? Dies wird auch automatisch geschehen.');">Einladung versenden</a>
                    {% endif %}
                {% else %}
                    {% if config.MAIL_ACTIVE %}
-                        <a class="btn btn-default" href="{{url_for("send_protocol_private", protocol_id=protocol.id)}}">Intern versenden</a>
+                        <a class="btn btn-default" href="{{url_for("send_protocol_private", protocol_id=protocol.id, csrf_token=get_csrf_token())}}">Intern versenden</a>
                        {% if protocol.public %}
-                            <a class="btn btn-default" href="{{url_for("send_protocol_public", protocol_id=protocol.id)}}">Öffentlich versenden</a>
+                            <a class="btn btn-default" href="{{url_for("send_protocol_public", protocol_id=protocol.id, csrf_token=get_csrf_token())}}">Öffentlich versenden</a>
                        {% endif %}
                    {% endif %}
                    {% if not protocol.public %}
-                        <a class="btn btn-default" href="{{url_for("publish_protocol", protocol_id=protocol.id)}}">Veröffentlichen</a>
+                        <a class="btn btn-default" href="{{url_for("publish_protocol", protocol_id=protocol.id, csrf_token=get_csrf_token())}}">Veröffentlichen</a>
                    {% endif %}
                {% endif %}
                <a class="btn btn-default" href="{{url_for("show_type", protocoltype_id=protocol.protocoltype.id)}}">Typ</a>
@@ -50,7 +50,7 @@
                    <a class="btn btn-success" href="{{url_for("download_document", document_id=protocol.get_compiled_document().id)}}">Download</a>
                {% endif %}
                {% if has_admin_right %}
-            <a class="btn btn-default" href="{{url_for("recompile_protocol", protocol_id=protocol.id)}}">Neu kompilieren</a>
+            <a class="btn btn-default" href="{{url_for("recompile_protocol", protocol_id=protocol.id, csrf_token=get_csrf_token())}}">Neu kompilieren</a>
            <a class="btn btn-danger" href="{{url_for("delete_protocol", protocol_id=protocol.id)}}" onclick="return confirm('Bist du dir sicher, dass du das Protokoll {{protocol.get_short_identifier()}} löschen möchtest?');">Löschen</a>
                {% endif %}
            {% endif %}
@@ -104,7 +104,7 @@
                                <li>
                                    {{decision.content}}
                                    {% if config.PRINTING_ACTIVE and has_private_view_right and decision.document is not none %}
-                                        <a href="{{url_for("print_decision", decisiondocument_id=decision.document.id)}}">Drucken</a>
+                                        <a href="{{url_for("print_decision", decisiondocument_id=decision.document.id, csrf_token=get_csrf_token())}}">Drucken</a>
                                    {% endif %}
                                    {{render_likes(decision.likes, decision_id=decision.id)}}</h2>
                                </li>

--- a/templates/protocol-tops-include.html
+++ b/templates/protocol-tops-include.html
@@ -27,9 +27,9 @@
            {% endif %}
            {% if not protocol.is_done() and has_modify_right %}
                <a href="{{url_for('edit_top', top_id=top.id)}}">Ändern</a>
-                <a href="{{url_for('move_top', top_id=top.id, diff=1)}}">Runter</a>
+                <a href="{{url_for('move_top', top_id=top.id, diff=1, csrf_token=get_csrf_token())}}">Runter</a>
-                <a href="{{url_for('move_top', top_id=top.id, diff=-1)}}">Hoch</a>
+                <a href="{{url_for('move_top', top_id=top.id, diff=-1, csrf_token=get_csrf_token())}}">Hoch</a>
-                <a href="{{url_for('delete_top', top_id=top.id)}}" onclick="return confirm('Bist du dir sicher, dass du den TOP {{top.name}} löschen möchtest?');">Löschen</a>
+                <a href="{{url_for('delete_top', top_id=top.id, csrf_token=get_csrf_token())}}" onclick="return confirm('Bist du dir sicher, dass du den TOP {{top.name}} löschen möchtest?');">Löschen</a>
            {% endif %}
            {% if has_private_view_right and top.description is not none and top.description|length > 0 %}
                <span class="glyphicon glyphicon-info-sign"></span>

--- a/utils.py
+++ b/utils.py
-from flask import request
+from flask import request, session
 import random
 import string
@@ -14,6 +14,8 @@ import ipaddress
 from socket import getfqdn
 from uuid import uuid4
 import subprocess
+import os
+import hashlib
 import config
@@ -258,3 +260,9 @@ def get_max_page_length_exp(objects):
 def get_internal_filename(protocol, document, filename):
    return "{}-{}-{}".format(protocol.id, document.id, filename)
+def get_csrf_token():
+    if "_csrf" not in session:
+        session["_csrf"] = hashlib.sha1(os.urandom(64)).hexdigest()
+    return session["_csrf"]
--- a/views/tables.py
+++ b/views/tables.py
 from flask import Markup, url_for, request
 from shared import date_filter, datetime_filter, time_filter, current_user
+from utils import get_csrf_token
 import config
@@ -341,7 +342,8 @@ class ProtocolTypeTable(SingleValueTable):
            ]))]
        action_part = [
            Table.link(
-                url_for("delete_type", protocoltype_id=self.value.id),
+                url_for("delete_type", protocoltype_id=self.value.id,
+                        csrf_token=get_csrf_token()),
                "Löschen",
                confirm="Bist du dir sicher, dass du den Protokolltype "
                        "{} löschen möchtest?".format(self.value.name))
@@ -371,10 +373,12 @@ class DefaultTOPsTable(Table):
            top.number,
            Table.concat([
                Table.link(
-                    url_for("move_default_top", defaulttop_id=top.id, diff=1),
+                    url_for("move_default_top", defaulttop_id=top.id, diff=1,
+                            csrf_token=get_csrf_token()),
                    "Runter"),
                Table.link(
-                    url_for("move_default_top", defaulttop_id=top.id, diff=-1),
+                    url_for("move_default_top", defaulttop_id=top.id, diff=-1,
+                            csrf_token=get_csrf_token()),
                    "Hoch"),
                Table.link(
                    url_for(
@@ -383,7 +387,8 @@ class DefaultTOPsTable(Table):
                        defaulttop_id=top.id),
                    "Ändern"),
                Table.link(
-                    url_for("delete_default_top", defaulttop_id=top.id),
+                    url_for("delete_default_top", defaulttop_id=top.id,
+                            csrf_token=get_csrf_token()),
                    "Löschen",
                    confirm="Bist du dir sicher, dass du den Standard-TOP "
                            "{} löschen willst?".format(top.name))
@@ -413,7 +418,8 @@ class MeetingRemindersTable(Table):
                url_for("edit_reminder", meetingreminder_id=reminder.id),
                "Ändern"),
            Table.link(
-                url_for("delete_reminder", meetingreminder_id=reminder.id),
+                url_for("delete_reminder", meetingreminder_id=reminder.id,
+                        csrf_token=get_csrf_token()),
                "Löschen",
                confirm="Bist du dir sicher, dass du die Einladungsmail {} "
                        "Tage vor der Sitzung löschen willst?".format(
@@ -452,7 +458,8 @@ class ErrorsTable(Table):
            datetime_filter(error.datetime),
            error.get_short_description(),
            Table.link(
-                url_for("delete_error", error_id=error.id, next=request.path),
+                url_for("delete_error", error_id=error.id,
+                        csrf_token=get_csrf_token()),
                "Löschen",
                confirm="Bist du dir sicher, dass du den Fehler löschen "
                        "möchtest?")
@@ -519,7 +526,8 @@ class TodosTable(Table):
        if todo.protocoltype.has_modify_right(user):
            row.append(Table.concat([
                Table.link(url_for("edit_todo", todo_id=todo.id), "Ändern"),
-                Table.link(url_for("delete_todo", todo_id=todo.id), "Löschen")
+                Table.link(url_for("delete_todo", todo_id=todo.id,
+                           csrf_token=get_csrf_token()), "Löschen")
            ]))
        else:
            row.append("")
@@ -552,7 +560,8 @@ class TodoTable(SingleValueTable):
                Table.link(
                    url_for("edit_todo", todo_id=self.value.id), "Ändern"),
                Table.link(
-                    url_for("delete_todo", todo_id=self.value.id), "Löschen",
+                    url_for("delete_todo", todo_id=self.value.id,
+                            csrf_token=get_csrf_token()), "Löschen",
                    confirm="Bist du dir sicher, dass du das Todo löschen "
                            "willst?")
            ]))
@@ -592,7 +601,8 @@ class DecisionsTable(Table):
            Table.link(
                url_for(
                    "print_decision",
-                    decisiondocument_id=decision.document.id),
+                    decisiondocument_id=decision.document.id,
+                    csrf_token=get_csrf_token()),
                "Drucken")
            if (config.PRINTING_ACTIVE
                and decision.protocol.protocoltype.has_modify_right(user)
@@ -634,11 +644,13 @@ class DocumentsTable(Table):
                "Bearbeiten"))
        if config.PRINTING_ACTIVE and document.protocol.has_modify_right(user):
            links.append(Table.link(
-                url_for("print_document", document_id=document.id),
+                url_for("print_document", document_id=document.id,
+                        csrf_token=get_csrf_token()),
                "Drucken"))
        if document.protocol.protocoltype.has_admin_right(user):
            links.append(Table.link(
-                url_for("delete_document", document_id=document.id),
+                url_for("delete_document", document_id=document.id,
+                        csrf_token=get_csrf_token()),
                "Löschen",
                confirm="Bist du dir sicher, dass du das Dokument {} löschen "
                        "willst?".format(document.name)))
@@ -675,7 +687,8 @@ class TodoMailsTable(Table):
                    url_for("edit_todomail", todomail_id=todomail.id),
                    "Ändern"),
                Table.link(
-                    url_for("delete_todomail", todomail_id=todomail.id),
+                    url_for("delete_todomail", todomail_id=todomail.id,
+                            csrf_token=get_csrf_token()),
                    "Löschen",
                    confirm="Bist du dir sicher, dass du die "
                            "Todomailzuordnung {} zu {} löschen "
@@ -707,7 +720,8 @@ class DefaultMetasTable(Table):
            Table.link(
                url_for("edit_defaultmeta", defaultmeta_id=meta.id), "Ändern"),
            Table.link(
-                url_for("delete_defaultmeta", defaultmeta_id=meta.id),
+                url_for("delete_defaultmeta", defaultmeta_id=meta.id,
+                        csrf_token=get_csrf_token()),
                "Löschen",
                confirm="Bist du dir sicher, dass du das Metadatenfeld {} "
                        "löschen willst?".format(meta.name))
@@ -739,7 +753,8 @@ class DecisionCategoriesTable(Table):
                Table.link(
                    url_for(
                        "delete_decisioncategory",
-                        decisioncategory_id=category.id),
+                        decisioncategory_id=category.id,
+                        csrf_token=get_csrf_token()),
                    "Löschen",
                    confirm="Bist du dir sicher, dass du die "
                            "Beschlusskategorie {} löschen "