From f2fa63848022453d35a8249184c8d56015121283 Mon Sep 17 00:00:00 2001 From: dgtlmoon Date: Mon, 21 Mar 2022 20:59:20 +0100 Subject: [PATCH] Security update - Protect against file:/// type access by webdriver/chrome. (#483) --- changedetectionio/fetch_site_status.py | 12 ++++++-- changedetectionio/tests/test_security.py | 36 ++++++++++++++++++++++++ changedetectionio/update_worker.py | 3 -- 3 files changed, 45 insertions(+), 6 deletions(-) create mode 100644 changedetectionio/tests/test_security.py diff --git a/changedetectionio/fetch_site_status.py b/changedetectionio/fetch_site_status.py index fa5660f8..64e45371 100644 --- a/changedetectionio/fetch_site_status.py +++ b/changedetectionio/fetch_site_status.py @@ -1,10 +1,10 @@ import hashlib +import os import re import time - import urllib3 -from inscriptis import get_text +from inscriptis import get_text from changedetectionio import content_fetcher, html_tools urllib3.disable_warnings(urllib3.exceptions.InsecureRequestWarning) @@ -24,8 +24,14 @@ class perform_site_check(): stripped_text_from_html = "" watch = self.datastore.data['watching'][uuid] - # Unset any existing notification error + # Protect against file:// access + if re.search(r'^file', watch['url'], re.IGNORECASE) and not os.getenv('ALLOW_FILE_URI', False): + raise Exception( + "file:// type access is denied for security reasons." + ) + + # Unset any existing notification error update_obj = {'last_notification_error': False, 'last_error': False} extra_headers = self.datastore.get_val(uuid, 'headers') diff --git a/changedetectionio/tests/test_security.py b/changedetectionio/tests/test_security.py new file mode 100644 index 00000000..b912da04 --- /dev/null +++ b/changedetectionio/tests/test_security.py @@ -0,0 +1,36 @@ +from flask import url_for +from . util import set_original_response, set_modified_response, live_server_setup +import time + +def test_setup(live_server): + live_server_setup(live_server) + +def test_file_access(client, live_server): + + res = client.post( + url_for("import_page"), + data={"urls": 'https://localhost'}, + follow_redirects=True + ) + + assert b"1 Imported" in res.data + + # Attempt to add a body with a GET method + res = client.post( + url_for("edit_page", uuid="first"), + data={ + "url": 'file:///etc/passwd', + "tag": "", + "method": "GET", + "fetch_backend": "html_requests", + "body": ""}, + follow_redirects=True + ) + time.sleep(3) + + res = client.get( + url_for("index", uuid="first"), + follow_redirects=True + ) + + assert b'denied for security reasons' in res.data diff --git a/changedetectionio/update_worker.py b/changedetectionio/update_worker.py index 9411f333..601eef45 100644 --- a/changedetectionio/update_worker.py +++ b/changedetectionio/update_worker.py @@ -42,7 +42,6 @@ class update_worker(threading.Thread): now = time.time() try: - changed_detected, update_obj, contents = update_handler.run(uuid) # Re #342 @@ -50,8 +49,6 @@ class update_worker(threading.Thread): # We then convert/.decode('utf-8') for the notification etc if not isinstance(contents, (bytes, bytearray)): raise Exception("Error - returned data from the fetch handler SHOULD be bytes") - - except PermissionError as e: self.app.logger.error("File permission error updating", uuid, str(e)) except content_fetcher.EmptyReply as e: