From 5483f5d694ffc0c480245a54bb7c688fa6748428 Mon Sep 17 00:00:00 2001
From: dgtlmoon <dgtlmoon@gmail.com>
Date: Mon, 21 Mar 2022 22:54:27 +0100
Subject: [PATCH] Security update - Use CSRF token protection for forms, make
 "remove password" use HTTP Post (#484)

---
 changedetectionio/__init__.py                 | 11 +++--
 changedetectionio/forms.py                    |  2 +
 changedetectionio/templates/edit.html         |  1 +
 changedetectionio/templates/import.html       |  1 +
 changedetectionio/templates/login.html        |  1 +
 changedetectionio/templates/scrub.html        |  1 +
 changedetectionio/templates/settings.html     | 14 +++---
 .../templates/watch-overview.html             |  1 +
 changedetectionio/tests/conftest.py           |  3 ++
 .../tests/test_access_control.py              | 44 ++++++++++++-------
 changedetectionio/tests/test_backend.py       |  1 +
 requirements.txt                              |  2 +-
 12 files changed, 54 insertions(+), 28 deletions(-)

diff --git a/changedetectionio/__init__.py b/changedetectionio/__init__.py
index 3ea9020c..f887c05c 100644
--- a/changedetectionio/__init__.py
+++ b/changedetectionio/__init__.py
@@ -35,6 +35,7 @@ from flask import (
     url_for,
 )
 from flask_login import login_required
+from flask_wtf import CSRFProtect
 
 from changedetectionio import html_tools
 
@@ -72,6 +73,9 @@ app.config['LOGIN_DISABLED'] = False
 # Disables caching of the templates
 app.config['TEMPLATES_AUTO_RELOAD'] = True
 
+csrf = CSRFProtect()
+csrf.init_app(app)
+
 notification_debug_log=[]
 
 def init_app_secret(datastore_path):
@@ -610,16 +614,15 @@ def changedetection_app(config=None, datastore_o=None):
             form.notification_format.data = datastore.data['settings']['application']['notification_format']
             form.base_url.data = datastore.data['settings']['application']['base_url']
 
-            # Password unset is a GET, but we can lock the session to always need the password
-            if not os.getenv("SALTED_PASS", False) and request.values.get('removepassword') == 'yes':
-                from pathlib import Path
+        if request.method == 'POST' and form.data.get('removepassword_button') == True:
+            # Password unset is a GET, but we can lock the session to a salted env password to always need the password
+            if not os.getenv("SALTED_PASS", False):
                 datastore.data['settings']['application']['password'] = False
                 flash("Password protection removed.", 'notice')
                 flask_login.logout_user()
                 return redirect(url_for('settings_page'))
 
         if request.method == 'POST' and form.validate():
-
             datastore.data['settings']['application']['notification_urls'] = form.notification_urls.data
             datastore.data['settings']['requests']['minutes_between_check'] = form.minutes_between_check.data
             datastore.data['settings']['application']['extract_title_as_title'] = form.extract_title_as_title.data
diff --git a/changedetectionio/forms.py b/changedetectionio/forms.py
index df5d6336..b3ffc547 100644
--- a/changedetectionio/forms.py
+++ b/changedetectionio/forms.py
@@ -353,3 +353,5 @@ class globalSettingsForm(commonSettingsForm):
     global_subtractive_selectors = StringListField('Remove elements', [ValidateCSSJSONXPATHInput(allow_xpath=False, allow_json=False)])
     global_ignore_text = StringListField('Ignore Text', [ValidateListRegex()])
     ignore_whitespace = BooleanField('Ignore whitespace')
+    save_button = SubmitField('Save', render_kw={"class": "pure-button pure-button-primary"})
+    removepassword_button = SubmitField('Remove password', render_kw={"class": "pure-button pure-button-primary"})
\ No newline at end of file
diff --git a/changedetectionio/templates/edit.html b/changedetectionio/templates/edit.html
index 0f10c8fa..74d41725 100644
--- a/changedetectionio/templates/edit.html
+++ b/changedetectionio/templates/edit.html
@@ -19,6 +19,7 @@
     <div class="box-wrap inner">
         <form class="pure-form pure-form-stacked"
               action="{{ url_for('edit_page', uuid=uuid, next = request.args.get('next') ) }}" method="POST">
+             <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
 
             <div class="tab-pane-inner" id="general">
                 <fieldset>
diff --git a/changedetectionio/templates/import.html b/changedetectionio/templates/import.html
index 943e580d..e9376fc4 100644
--- a/changedetectionio/templates/import.html
+++ b/changedetectionio/templates/import.html
@@ -4,6 +4,7 @@
 <div class="edit-form">
      <div class="inner">
         <form class="pure-form pure-form-aligned" action="{{url_for('import_page')}}" method="POST">
+            <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
             <fieldset class="pure-group">
               <legend>
                 Enter one URL per line, and optionally add tags for each URL after a space, delineated by comma (,):
diff --git a/changedetectionio/templates/login.html b/changedetectionio/templates/login.html
index 32259930..2e24ddb4 100644
--- a/changedetectionio/templates/login.html
+++ b/changedetectionio/templates/login.html
@@ -4,6 +4,7 @@
 <div class="login-form">
  <div class="inner">
     <form class="pure-form pure-form-stacked" action="{{url_for('login')}}" method="POST">
+        <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
         <fieldset>
             <div class="pure-control-group">
                 <label for="password">Password</label>
diff --git a/changedetectionio/templates/scrub.html b/changedetectionio/templates/scrub.html
index 25ddbf18..bd006b31 100644
--- a/changedetectionio/templates/scrub.html
+++ b/changedetectionio/templates/scrub.html
@@ -4,6 +4,7 @@
 <div class="edit-form">
     <div class="box-wrap inner">
     <form class="pure-form pure-form-stacked" action="{{url_for('scrub_page')}}" method="POST">
+        <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
         <fieldset>
             <div class="pure-control-group">
                 This will remove all version snapshots/data, but keep your list of URLs. <br/>
diff --git a/changedetectionio/templates/settings.html b/changedetectionio/templates/settings.html
index 9551695f..129e5353 100644
--- a/changedetectionio/templates/settings.html
+++ b/changedetectionio/templates/settings.html
@@ -1,7 +1,7 @@
 {% extends 'base.html' %}
 
 {% block content %}
-{% from '_helpers.jinja' import render_field %}
+{% from '_helpers.jinja' import render_field, render_button %}
 {% from '_common_fields.jinja' import render_common_settings_form %}
 
 <script type="text/javascript" src="{{url_for('static_content', group='js', filename='settings.js')}}" defer></script>
@@ -18,6 +18,7 @@
     </div>
     <div class="box-wrap inner">
         <form class="pure-form pure-form-stacked settings" action="{{url_for('settings_page')}}" method="POST">
+            <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
             <div class="tab-pane-inner" id="general">
                 <fieldset>
                     <div class="pure-control-group">
@@ -27,8 +28,7 @@
                     <div class="pure-control-group">
                         {% if not hide_remove_pass %}
                             {% if current_user.is_authenticated %}
-                            <a href="{{url_for('settings_page', removepassword='yes')}}"
-                               class="pure-button pure-button-primary">Remove password</a>
+                                {{ render_button(form.removepassword_button) }}
                             {% else %}
                             {{ render_field(form.password) }}
                             <span class="pure-form-message-inline">Password protection for your changedetection.io application.</span>
@@ -114,11 +114,9 @@ nav
 
             <div id="actions">
                 <div class="pure-control-group">
-                    <button type="submit" class="pure-button pure-button-primary">Save</button>
-                                           <a href="{{url_for('index')}}" class="pure-button button-small button-cancel">Back</a>
-                        <a href="{{url_for('scrub_page')}}" class="pure-button button-small button-cancel">Delete
-                            History
-                            Snapshot Data</a>
+                    {{ render_button(form.save_button) }}
+                    <a href="{{url_for('index')}}" class="pure-button button-small button-cancel">Back</a>
+                    <a href="{{url_for('scrub_page')}}" class="pure-button button-small button-cancel">Delete History Snapshot Data</a>
                 </div>
             </div>
         </form>
diff --git a/changedetectionio/templates/watch-overview.html b/changedetectionio/templates/watch-overview.html
index bf222dbf..313c1bf5 100644
--- a/changedetectionio/templates/watch-overview.html
+++ b/changedetectionio/templates/watch-overview.html
@@ -5,6 +5,7 @@
 <div class="box">
 
     <form class="pure-form" action="{{ url_for('api_watch_add') }}" method="POST" id="new-watch-form">
+        <input type="hidden" name="csrf_token" value="{{ csrf_token() }}"/>
         <fieldset>
             <legend>Add a new change detection watch</legend>
                 {{ render_simple_field(form.url, placeholder="https://...", required=true) }}
diff --git a/changedetectionio/tests/conftest.py b/changedetectionio/tests/conftest.py
index aced3075..98d4dd9e 100644
--- a/changedetectionio/tests/conftest.py
+++ b/changedetectionio/tests/conftest.py
@@ -42,6 +42,9 @@ def app(request):
     cleanup(app_config['datastore_path'])
     datastore = store.ChangeDetectionStore(datastore_path=app_config['datastore_path'], include_default_watches=False)
     app = changedetection_app(app_config, datastore)
+
+    # Disable CSRF while running tests
+    app.config['WTF_CSRF_ENABLED'] = False
     app.config['STOP_THREADS'] = True
 
     def teardown():
diff --git a/changedetectionio/tests/test_access_control.py b/changedetectionio/tests/test_access_control.py
index 026929ba..80eeb780 100644
--- a/changedetectionio/tests/test_access_control.py
+++ b/changedetectionio/tests/test_access_control.py
@@ -4,8 +4,8 @@ from flask import url_for
 def test_check_access_control(app, client):
     # Still doesnt work, but this is closer.
 
-    with app.test_client() as c:
-        # Check we dont have any password protection enabled yet.
+    with app.test_client(use_cookies=True) as c:
+        # Check we don't have any password protection enabled yet.
         res = c.get(url_for("settings_page"))
         assert b"Remove password" not in res.data
 
@@ -46,15 +46,20 @@ def test_check_access_control(app, client):
         assert b"BACKUP" in res.data
         assert b"IMPORT" in res.data
         assert b"LOG OUT" in res.data
+        assert b"minutes_between_check" in res.data
+        assert b"fetch_backend" in res.data
 
-        # Now remove the password so other tests function, @todo this should happen before each test automatically
-        res = c.get(url_for("settings_page", removepassword="yes"),
-              follow_redirects=True)
-        assert b"Password protection removed." in res.data
-
-        res = c.get(url_for("index"))
-        assert b"LOG OUT" not in res.data
-
+        res = c.post(
+            url_for("settings_page"),
+            data={
+                "minutes_between_check": 180,
+                "tag": "",
+                "headers": "",
+                "fetch_backend": "html_webdriver",
+                "removepassword_button": "Remove password"
+            },
+            follow_redirects=True,
+        )
 
 # There was a bug where saving the settings form would submit a blank password
 def test_check_access_control_no_blank_password(app, client):
@@ -71,8 +76,7 @@ def test_check_access_control_no_blank_password(app, client):
             data={"password": "",
                   "minutes_between_check": 180,
                   'fetch_backend': "html_requests"},
-
-        follow_redirects=True
+            follow_redirects=True
         )
 
         assert b"Password protection enabled." not in res.data
@@ -91,7 +95,8 @@ def test_check_access_no_remote_access_to_remove_password(app, client):
         # Enable password check.
         res = c.post(
             url_for("settings_page"),
-            data={"password": "password", "minutes_between_check": 180,
+            data={"password": "password",
+                  "minutes_between_check": 180,
                   'fetch_backend': "html_requests"},
             follow_redirects=True
         )
@@ -99,8 +104,17 @@ def test_check_access_no_remote_access_to_remove_password(app, client):
         assert b"Password protection enabled." in res.data
         assert b"Login" in res.data
 
-        res = c.get(url_for("settings_page", removepassword="yes"),
-              follow_redirects=True)
+        res = c.post(
+            url_for("settings_page"),
+            data={
+                "minutes_between_check": 180,
+                "tag": "",
+                "headers": "",
+                "fetch_backend": "html_webdriver",
+                "removepassword_button": "Remove password"
+            },
+            follow_redirects=True,
+        )
         assert b"Password protection removed." not in res.data
 
         res = c.get(url_for("index"),
diff --git a/changedetectionio/tests/test_backend.py b/changedetectionio/tests/test_backend.py
index b279251f..ddd32751 100644
--- a/changedetectionio/tests/test_backend.py
+++ b/changedetectionio/tests/test_backend.py
@@ -25,6 +25,7 @@ def test_check_basic_change_detection_functionality(client, live_server):
         data={"urls": url_for('test_endpoint', _external=True)},
         follow_redirects=True
     )
+
     assert b"1 Imported" in res.data
 
     time.sleep(sleep_time_for_fetch_thread)
diff --git a/requirements.txt b/requirements.txt
index fb7a0a76..feef375b 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -1,5 +1,5 @@
 flask~= 2.0
-
+flask_wtf
 eventlet>=0.31.0
 validators
 timeago ~=1.0