diff --git a/.github/workflows/test-features.yml b/.github/workflows/test-features.yml index 20d0b0a4..306566e4 100644 --- a/.github/workflows/test-features.yml +++ b/.github/workflows/test-features.yml @@ -32,6 +32,7 @@ on: - 'AppImage/scripts/flask_proxmenux_routes.py' - 'AppImage/scripts/guest_backup_archives.py' - 'AppImage/scripts/flask_server.py' + - 'AppImage/scripts/tests/test_static_path_containment.py' - 'AppImage/scripts/ovs_topology.py' - 'AppImage/scripts/tests/test_ovs_topology.py' - 'AppImage/components/network-flow.tsx' @@ -98,6 +99,7 @@ on: - 'AppImage/scripts/flask_proxmenux_routes.py' - 'AppImage/scripts/guest_backup_archives.py' - 'AppImage/scripts/flask_server.py' + - 'AppImage/scripts/tests/test_static_path_containment.py' - 'AppImage/scripts/ovs_topology.py' - 'AppImage/scripts/tests/test_ovs_topology.py' - 'AppImage/components/network-flow.tsx' @@ -189,6 +191,21 @@ jobs: NODE_PATH: ${{ github.workspace }}/.github/ci/offline-node/node_modules run: node tests/test_backup_storage_retention.cjs + static-files: + runs-on: ubuntu-latest + timeout-minutes: 5 + steps: + - uses: actions/checkout@v4 + with: + persist-credentials: false + - uses: actions/setup-python@v5 + with: + python-version: '3.11' + - name: Install Flask + run: python -m pip install 'Flask==3.1.2' + - name: Test that the Monitor's own files never leave their folder + run: python -B -m unittest discover -s AppImage/scripts/tests -p test_static_path_containment.py -v + nvidia-installer: runs-on: ubuntu-latest timeout-minutes: 5 diff --git a/AppImage/scripts/flask_server.py b/AppImage/scripts/flask_server.py index 43182d71..0a76b58e 100644 --- a/AppImage/scripts/flask_server.py +++ b/AppImage/scripts/flask_server.py @@ -3850,14 +3850,8 @@ def serve_next_static(filename): appimage_root = os.path.dirname(base_dir) static_dir = os.path.join(appimage_root, 'web', '_next') - file_path = os.path.join(static_dir, filename) - - if os.path.exists(file_path): - return send_file(file_path) - - # print(f"[v0] ❌ Next.js static file not found: {file_path}") - pass - return '', 404 + # send_from_directory refuses any path that leaves static_dir. + return send_from_directory(static_dir, filename) except Exception as e: # print(f"Error serving Next.js static file {filename}: {e}") pass diff --git a/AppImage/scripts/tests/test_static_path_containment.py b/AppImage/scripts/tests/test_static_path_containment.py new file mode 100644 index 00000000..c11df130 --- /dev/null +++ b/AppImage/scripts/tests/test_static_path_containment.py @@ -0,0 +1,83 @@ +"""The routes that serve the Monitor's own files never return a file from +outside their folder, whatever the request path says.""" +import ast +import os +from pathlib import Path +import tempfile +import unittest +from unittest.mock import patch + +from flask import Flask, jsonify, send_file, send_from_directory + +SCRIPTS = Path(__file__).resolve().parents[1] +HANDLERS = {'serve_next_static', 'serve_static_files', 'serve_images'} +ESCAPES = ( + '../../secret.txt', '..%2f..%2fsecret.txt', '..%2F..%2Fsecret.txt', + '%2e%2e/%2e%2e/secret.txt', '....//....//secret.txt', '/../../secret.txt', + '..\\..\\secret.txt', '%2e%2e%2f%2e%2e%2fsecret.txt', +) + + +def app_with_handlers(): + """The real route functions of flask_server.py on a bare Flask app.""" + tree = ast.parse((SCRIPTS / 'flask_server.py').read_text()) + app = Flask(__name__) + namespace = {'os': os, 'app': app, 'send_file': send_file, + 'send_from_directory': send_from_directory, 'jsonify': jsonify} + nodes = [n for n in tree.body if isinstance(n, ast.FunctionDef) and n.name in HANDLERS] + if {n.name for n in nodes} != HANDLERS: + raise AssertionError('static handlers not found') + exec(compile(ast.Module(body=nodes, type_ignores=[]), 'flask_server.py', 'exec'), namespace) + for node in nodes: + for decorator in node.decorator_list: + rule = decorator.args[0].value + app.add_url_rule(rule, node.name, namespace[node.name]) + return app + + +class StaticPathContainment(unittest.TestCase): + def setUp(self): + temp = tempfile.TemporaryDirectory() + self.addCleanup(temp.cleanup) + root = Path(temp.name) + self.appdir = root / 'appdir' + (self.appdir / 'web/_next/static').mkdir(parents=True) + (self.appdir / 'web/images').mkdir(parents=True) + (self.appdir / 'web/_next/static/app.js').write_text('next-asset') + (self.appdir / 'web/images/logo.png').write_text('image-asset') + (self.appdir / 'web/index.txt').write_text('web-asset') + # Harmless markers just outside each served folder and at the root. + for marker in (root / 'secret.txt', self.appdir / 'secret.txt', self.appdir / 'web/secret.txt'): + marker.write_text('outside-marker') + patcher = patch.dict(os.environ, {'APPDIR': str(self.appdir)}) + patcher.start() + self.addCleanup(patcher.stop) + self.client = app_with_handlers().test_client() + + def test_files_inside_the_folders_are_served(self): + for path, body in (('/_next/static/app.js', b'next-asset'), ('/images/logo.png', b'image-asset'), + ('/index.txt', b'web-asset')): + with self.subTest(path=path): + response = self.client.get(path) + self.assertEqual((response.status_code, response.get_data()), (200, body)) + response.close() + + def test_no_path_leaves_the_folder(self): + for prefix in ('/_next/', '/images/', '/'): + for escape in ESCAPES: + if prefix == '/' and escape.startswith('/'): + continue # '//' would be read as a host by the test client + with self.subTest(path=prefix + escape): + response = self.client.get(prefix + escape) + self.assertNotIn(b'outside-marker', response.get_data()) + self.assertNotEqual(response.status_code, 200) + response.close() + + def test_an_absolute_path_is_not_served(self): + response = self.client.get('/_next/' + str(self.appdir / 'secret.txt')) + self.assertNotIn(b'outside-marker', response.get_data()) + response.close() + + +if __name__ == '__main__': + unittest.main()