From b2ea97b479f860c5b8b3f945d841957daa1758f4 Mon Sep 17 00:00:00 2001 From: claravox Date: Mon, 11 Dec 2023 13:26:44 +0100 Subject: [PATCH] YDA-5546: harden static loader --- yoda_eus/app.py | 17 +++---- yoda_eus/tests/test_unit.py | 91 +++++++++++++++++++++++++++++++++++-- yoda_eus/util.py | 43 ++++++++++++++++++ 3 files changed, 136 insertions(+), 15 deletions(-) create mode 100644 yoda_eus/util.py diff --git a/yoda_eus/app.py b/yoda_eus/app.py index 5960706..8030c4c 100644 --- a/yoda_eus/app.py +++ b/yoda_eus/app.py @@ -7,7 +7,7 @@ import urllib.parse from datetime import datetime from os import path -from typing import Any, Dict +from typing import Any, Dict, Optional import bcrypt from flask import abort, Flask, jsonify, make_response, render_template, request, Response, send_from_directory @@ -17,6 +17,7 @@ from jinja2 import ChoiceLoader, FileSystemLoader from yoda_eus.mail import is_email_valid, send_email_template_if_needed from yoda_eus.password_complexity import check_password_complexity +from yoda_eus.util import get_validated_static_path db = SQLAlchemy() @@ -578,7 +579,7 @@ def check_api_secret() -> Response: abort(make_response(jsonify({'status': 'error', 'message': 'EUS secret header not present or does not match.'}), 403)) @app.before_request - def static_loader() -> Response: + def static_loader() -> Optional[Response]: """ Static files handling - recognisable through '/assets/' Override requested static file if present in user_static_area @@ -589,15 +590,9 @@ def static_loader() -> Response: :returns: Static file """ - if request.full_path.split('/')[1] == 'assets': - user_static_area = path.join(app.config.get('YODA_THEME_PATH'), app.config.get('YODA_THEME')) - asset_dir, asset_name = path.split(request.path) - static_dir = asset_dir.replace('/assets', user_static_area + '/static') - user_static_filename = path.join(static_dir, asset_name) - - if not path.exists(user_static_filename): - static_dir = asset_dir.replace('/assets', '/var/www/yoda/static') - + result = get_validated_static_path(request.full_path, request.path, app.config.get('YODA_THEME_PATH'), app.config.get('YODA_THEME')) + if result is not None: + static_dir, asset_name = result return send_from_directory(static_dir, asset_name) @ app.url_defaults diff --git a/yoda_eus/tests/test_unit.py b/yoda_eus/tests/test_unit.py index a01a310..0624148 100644 --- a/yoda_eus/tests/test_unit.py +++ b/yoda_eus/tests/test_unit.py @@ -1,14 +1,15 @@ -__copyright__ = 'Copyright (c) 2023, Utrecht University' -__license__ = 'GPLv3, see LICENSE' +__copyright__ = "Copyright (c) 2023, Utrecht University" +__license__ = "GPLv3, see LICENSE" import string +from unittest.mock import patch from yoda_eus.mail import is_email_valid from yoda_eus.password_complexity import check_password_complexity +from yoda_eus.util import get_validated_static_path class TestMain: - def test_password_validation_ok(self): result = check_password_complexity("Test123456789!") assert len(result) == 0 @@ -50,10 +51,92 @@ def test_password_validation_multiple(self): assert len(result) == 3 assert "Password is too short: it needs to be at least 10 characters." in result assert "Password needs to contain at least one digit." in result - assert "Password needs to contain at least one punctuation character ({})".format(string.punctuation) in result + assert ( + "Password needs to contain at least one punctuation character ({})".format( + string.punctuation + ) + in result + ) def is_email_valid_yes(self): assert is_email_valid("yoda@uu.nl") def is_email_valid_no(self): assert not is_email_valid("this is not a valid email address") + + def exists_return_value(self, pathname): + """ Mock path.exists function. True if path does not contain "theme" and "uu" """ + return not ("theme" in pathname and "uu" in pathname) + + @patch("os.path.exists") + def test_static_loader_valid_path(self, mock_exists): + mock_exists.side_effect = self.exists_return_value + # uu theme + static_dir, asset_name = get_validated_static_path( + "/assets/img/logo.svg?wekr", + "/assets/img/logo.svg", + "/var/www/yoda/themes", + "uu", + ) + assert static_dir == "/var/www/yoda/static/img" + assert asset_name == "logo.svg" + # other theme + static_dir, asset_name = get_validated_static_path( + "/assets/img/logo.svg?wekr", + "/assets/img/logo.svg", + "/var/www/yoda/themes", + "wur", + ) + assert static_dir == "/var/www/yoda/themes/wur/static/img" + assert asset_name == "logo.svg" + + @patch("os.path.exists") + def test_static_loader_invalid_path(self, mock_exists): + mock_exists.side_effect = self.exists_return_value + # Too short + assert ( + get_validated_static_path("/?sawerw", "/", "/var/www/yoda/themes", "uu") + is None + ) + # Path traversal attack + assert ( + get_validated_static_path( + "/assets/../../../../etc/passwd?werwrwr", + "/assets/../../../../etc/passwd", + "/var/www/yoda/themes", + "uu", + ) + is None + ) + # non-printable characters + full_path = "/assets/" + chr(13) + "img/logo.svg?werwer" + path = "/assets/" + chr(13) + "img/logo.svg" + assert ( + get_validated_static_path(full_path, path, "/var/www/yoda/themes", "uu") + is None + ) + assert ( + get_validated_static_path(full_path, path, "/var/www/yoda/themes", "wur") + is None + ) + # non-printable characters in asset name + full_path = "/assets/img/l" + chr(13) + "ogo.svg?werwer" + path = "/assets/img/l" + chr(13) + "ogo.svg" + assert ( + get_validated_static_path(full_path, path, "/var/www/yoda/themes", "uu") + is None + ) + assert ( + get_validated_static_path(full_path, path, "/var/www/yoda/themes", "wur") + is None + ) + # .. in file name + assert ( + get_validated_static_path( + "/assets/img/lo..go.svg?sklaerw", + "/assets/img/lo..go.svg?sklaerw", + "/var/www/yoda/themes", + "uu", + ) + is None + ) diff --git a/yoda_eus/util.py b/yoda_eus/util.py new file mode 100644 index 0000000..71b0d8b --- /dev/null +++ b/yoda_eus/util.py @@ -0,0 +1,43 @@ +#!/usr/bin/env python3 + +__copyright__ = 'Copyright (c) 2021-2023, Utrecht University' +__license__ = 'GPLv3, see LICENSE' + +from typing import Optional, Tuple +from os import path +from re import fullmatch +from werkzeug.security import safe_join +from werkzeug.utils import secure_filename + + +def get_validated_static_path(full_path, request_path, yoda_theme_path, yoda_theme) -> Optional[Tuple[str, str]]: + """ + Static files handling - recognisable through '/assets/' + Confirms that input path is valid and return corresponding static path + + :param full_path: Full path of request + :param request_path: Short path of request + :param yoda_theme_path: Path to the yoda themes + :param yoda_theme: Name of the chosen theme + + :returns: Tuple of static directory and filename for correct path, None for incorrect path + """ + parts = full_path.split('/') + + if len(parts) > 2 and fullmatch('[ -~]*', full_path) is not None and parts[1] == 'assets': + parts = parts[2:-1] + user_static_area = path.join(yoda_theme_path, yoda_theme) + _, asset_name = path.split(request_path) + # Confirm that asset_name is safe + if asset_name != secure_filename(asset_name): + return + + static_dir = safe_join(user_static_area + '/static', *parts) + if not static_dir: + return + user_static_filename = path.join(static_dir, asset_name) + + if not path.exists(user_static_filename): + static_dir = safe_join('/var/www/yoda/static', *parts) + + return static_dir, asset_name