CVE-2021-4162: Cross-Site Request Forgery (CSRF) (CWE-352)
value: archivy 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: 4.3
Published: 2021-12-25T12:15Z
Repository: https://github.com/archivy/archivy
Language: Python
Files changed: archivy/__init__.py, archivy/click_web/resources/cmd_exec.py, archivy/routes.py, archivy/templates/click_web/command_form.html, archivy/templates/dataobjs/show.html, tests/functional/test_routes.py
Commit date: 2021-12-24 12:16:44 +0100
Commit message: better CSRF protection; change delete route to POST

Diff: diff --git a/archivy/__init__.py b/archivy/__init__.py
index b3aa3e7..a4eaeee 100644
--- a/archivy/__init__.py
+++ b/archivy/__init__.py
@@ -4,10 +4,11 @@ from shutil import which
 
 from elasticsearch.exceptions import RequestError
 from flask import Flask
 from flask_compress import Compress
 from flask_login import LoginManager
+from flask_wtf.csrf import CSRFProtect
 
 from archivy import helpers
 from archivy.api import api_bp
 from archivy.models import User
 from archivy.config import Config
@@ -75,10 +76,11 @@ if app.config["SEARCH_CONF"]["enabled"]:
 # login routes / setup
 login_manager = LoginManager()
 login_manager.login_view = "login"
 login_manager.init_app(app)
 app.register_blueprint(api_bp, url_prefix="/api")
+csrf = CSRFProtect(app)
 
 # compress files
 Compress(app)
 
 
diff --git a/archivy/click_web/resources/cmd_exec.py b/archivy/click_web/resources/cmd_exec.py
index 3e31f89..c92e9d0 100644
--- a/archivy/click_web/resources/cmd_exec.py
+++ b/archivy/click_web/resources/cmd_exec.py
@@ -135,14 +135,12 @@ def _get_download_link(field_info):
     return f'<a href="{uri}">{field_info.link_name}</a>'
 
 
 class RequestToCommandArgs:
     def __init__(self):
-        field_infos = [
-            FieldInfo.factory(key)
-            for key in list(request.form.keys()) + list(request.files.keys())
-        ]
+        keys = [key for key in list(request.form.keys()) + list(request.files.keys())]
+        field_infos = [FieldInfo.factory(key) for key in keys if key != "csrf_token"]
         # important to sort them so they will be in expected order on command line
         self.field_infos = list(sorted(field_infos))
 
     def command_args(self, command_index) -> List[str]:
         """
diff --git a/archivy/routes.py b/archivy/routes.py
index 1c623a5..dc7a368 100644
--- a/archivy/routes.py
+++ b/archivy/routes.py
@@ -230,11 +230,11 @@ def move_item(dataobj_id):
     except FileExistsError:
         flash("Data already in target directory.", "error")
         return redirect(f"/dataobj/{dataobj_id}")
 
 
-@app.route("/dataobj/delete/<int:dataobj_id>", methods=["DELETE", "GET"])
+@app.route("/dataobj/delete/<int:dataobj_id>", methods=["POST"])
 def delete_data(dataobj_id):
     try:
         data.delete_item(dataobj_id)
     except BaseException:
         flash("Data could not be found!", "error")
diff --git a/archivy/templates/click_web/command_form.html b/archivy/templates/click_web/command_form.html
index cdffd0c..3c68052 100644
--- a/archivy/templates/click_web/command_form.html
+++ b/archivy/templates/click_web/command_form.html
@@ -27,10 +27,11 @@
                     {% endif %}
                 </div>
             </div>
         {% endfor %}
     {% endfor %}
+	<input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
     <button type="submit" id="submit_btn" class="btn btn-primary m-2">Run</button>
 </form>
 
 <div id="output-header" hidden="true"></div>
 <div class="script-output" id="output" hidden="true"></div>
diff --git a/archivy/templates/dataobjs/show.html b/archivy/templates/dataobjs/show.html
index aad8663..8378192 100644
--- a/archivy/templates/dataobjs/show.html
+++ b/archivy/templates/dataobjs/show.html
@@ -69,11 +69,11 @@
       <div class="d-flex" id="post-btns">
           <button onclick="toggleEditor(this)" class="btn">
             <svg class="octicon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" width="16" height="16"><path fill-rule="evenodd" d="M11.013 1.427a1.75 1.75 0 012.474 0l1.086 1.086a1.75 1.75 0 010 2.474l-8.61 8.61c-.21.21-.47.364-.756.445l-3.251.93a.75.75 0 01-.927-.928l.929-3.25a1.75 1.75 0 01.445-.758l8.61-8.61zm1.414 1.06a.25.25 0 00-.354 0L10.811 3.75l1.439 1.44 1.263-1.263a.25.25 0 000-.354l-1.086-1.086zM11.189 6.25L9.75 4.81l-6.286 6.287a.25.25 0 00-.064.108l-.558 1.953 1.953-.558a.249.249 0 00.108-.064l6.286-6.286z"></path></svg>
             <span>Edit</span>
           </button>
-          <form action="/dataobj/delete/{{ dataobj['id'] }}" method="delete" onsubmit="return confirm('Delete this item permanently?')" novalidate>
+          <form action="/dataobj/delete/{{ dataobj['id'] }}" method="POST" onsubmit="return confirm('Delete this item permanently?')" novalidate>
             {{ form.hidden_tag() }}
             <button class="btn btn-delete">
               <svg class="octicon" xmlns="http://www.w3.org/2000/svg" viewBox="0 0 16 16" width="16" height="16"><path fill-rule="evenodd" d="M6.5 1.75a.25.25 0 01.25-.25h2.5a.25.25 0 01.25.25V3h-3V1.75zm4.5 0V3h2.25a.75.75 0 010 1.5H2.75a.75.75 0 010-1.5H5V1.75C5 .784 5.784 0 6.75 0h2.5C10.216 0 11 .784 11 1.75zM4.496 6.675a.75.75 0 10-1.492.15l.66 6.6A1.75 1.75 0 005.405 15h5.19c.9 0 1.652-.681 1.741-1.576l.66-6.6a.75.75 0 00-1.492-.149l-.66 6.6a.25.25 0 01-.249.225h-5.19a.25.25 0 01-.249-.225l-.66-6.6z"></path></svg>
               <span>Delete</span>
             </button>
diff --git a/tests/functional/test_routes.py b/tests/functional/test_routes.py
index 836f38b..7e59bf4 100644
--- a/tests/functional/test_routes.py
+++ b/tests/functional/test_routes.py
@@ -59,16 +59,16 @@ def test_get_dataobj(test_app, client: FlaskClient, note_fixture):
     response = client.get("/dataobj/1")
     assert response.status_code == 200
 
 
 def test_get_delete_dataobj_not_found(test_app, client: FlaskClient):
-    response = client.get("/dataobj/delete/1")
+    response = client.post("/dataobj/delete/1")
     assert response.status_code == 302
 
 
 def test_get_delete_dataobj(test_app, client: FlaskClient, note_fixture):
-    response = client.get("/dataobj/delete/1")
+    response = client.post("/dataobj/delete/1")
     assert response.status_code == 302
 
 
 def test_create_new_bookmark(
     test_app, client: FlaskClient, mocked_responses: RequestsMock

Vulnerable code:         field_infos = [
            FieldInfo.factory(key)
            for key in list(request.form.keys()) + list(request.files.keys())
        ]
@app.route("/dataobj/delete/<int:dataobj_id>", methods=["DELETE", "GET"])
          <form action="/dataobj/delete/{{ dataobj['id'] }}" method="delete" onsubmit="return confirm('Delete this item permanently?')" novalidate>
    response = client.get("/dataobj/delete/1")
    response = client.get("/dataobj/delete/1")
Fixed code: from flask_wtf.csrf import CSRFProtect
csrf = CSRFProtect(app)
        keys = [key for key in list(request.form.keys()) + list(request.files.keys())]
        field_infos = [FieldInfo.factory(key) for key in keys if key != "csrf_token"]
@app.route("/dataobj/delete/<int:dataobj_id>", methods=["POST"])
	<input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
          <form action="/dataobj/delete/{{ dataobj['id'] }}" method="POST" onsubmit="return confirm('Delete this item permanently?')" novalidate>
    response = client.post("/dataobj/delete/1")
    response = client.post("/dataobj/delete/1")