CVE-2024-39903: Insufficient Information (NVD-CWE-noinfo)
value: Solara is a pure Python, React-style framework for scaling Jupyter and web apps. A Local File Inclusion (LFI) vulnerability was identified in widgetti/solara, in version <1.35.1, which was fixed in version 1.35.1. This vulnerability arises from the application's failure to properly validate URI fragments for directory traversal sequences such as '../' when serving static files. An attacker can exploit this flaw by manipulating the fragment part of the URI to read arbitrary files on the local file system.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-07-12T15:15Z
Repository: https://github.com/widgetti/solara
Language: Python
Files changed: solara/server/cdn_helper.py, solara/server/flask.py, solara/server/starlette.py, solara/server/utils.py, tests/integration/apps/not-allowed, tests/integration/apps/public/test.txt, tests/integration/apps/secure/app.py, tests/integration/server_test.py
Commit date: 2024-07-11 17:41:02 +0200
Commit message: fix: CVE followup, do not serve relative files

We still served files above the directory of intent.
We now test all endpoint, using both starlette and flask to
make sure this will not happen again.

Diff: diff --git a/solara/server/cdn_helper.py b/solara/server/cdn_helper.py
index 614cc18..69c085a 100644
--- a/solara/server/cdn_helper.py
+++ b/solara/server/cdn_helper.py
@@ -1,21 +1,24 @@
 import logging
-import os
 import pathlib
 import shutil
 
 import requests
+from solara.server.utils import path_is_child_of
 
 import solara.settings
 
 logger = logging.getLogger("Solara.cdn")
 
 cdn_url_path = "_solara/cdn"
 
 
 def put_in_cache(base_cache_dir: pathlib.Path, path, data: bytes):
     cache_path = base_cache_dir / path
+    if not path_is_child_of(cache_path, base_cache_dir):
+        raise PermissionError("Trying to write outside of cache directory")
+
     pathlib.Path(cache_path.parent).mkdir(parents=True, exist_ok=True)
     try:
         logger.info("Writing cache file: %s", cache_path)
         cache_path.write_bytes(data)
     except FileNotFoundError:
@@ -25,20 +28,12 @@ def put_in_cache(base_cache_dir: pathlib.Path, path, data: bytes):
 def get_from_cache(base_cache_dir: pathlib.Path, path):
     cache_path = pathlib.Path(base_cache_dir / path)
     # Make sure cache_path is a subdirectory of base_cache_dir
     # so we don't accidentally read files from the parent directory
     # which is a security risk.
-    # We use os.path.normpath() because we do not want to follow symlinks
-    # in editable installs, since some packages are symlinked
-    if not os.path.normpath(cache_path).startswith(os.path.normpath(base_cache_dir)):
-        logger.warning(
-            "Trying to read from outside of cache directory: %s is not a subdir of %s (%s - %s)",
-            cache_path,
-            base_cache_dir,
-            os.path.normpath(cache_path),
-            os.path.normpath(base_cache_dir),
-        )
+    if not path_is_child_of(cache_path, base_cache_dir):
+        logger.warning("Trying to read from outside of cache directory: %s is not a subdir of %s", cache_path, base_cache_dir)
         raise PermissionError("Trying to read from outside of cache directory")
 
     try:
         logger.info("Opening cache file: %s", cache_path)
         return cache_path.read_bytes()
@@ -72,10 +67,13 @@ def get_data(base_cache_dir: pathlib.Path, path):
 def get_path(base_cache_dir: pathlib.Path, path) -> pathlib.Path:
     parts = path.replace("\\", "/").split("/")
     store_path = path if len(parts) != 1 else pathlib.Path(path) / "__main.js"
     cache_path = base_cache_dir / store_path
 
+    if not path_is_child_of(cache_path, base_cache_dir):
+        raise PermissionError("Trying to read from outside of cache directory")
+
     if cache_path.exists():
         # before d7eba856f100d5c3c64f4eec22c62390f084cb40 on windows, we could
         # accidentally write to the cache directory, so we need to check if we still
         # have an old directory layout, and remove that first.
         if cache_path.is_dir():
diff --git a/solara/server/flask.py b/solara/server/flask.py
index 6b659a5..d8f48e2 100644
--- a/solara/server/flask.py
+++ b/solara/server/flask.py
@@ -197,11 +197,11 @@ def nbext(dir, filename):
     if not allowed():
         abort(401)
     for directory in server.nbextensions_directories:
         file = directory / dir / filename
         if file.exists():
-            return send_from_directory(directory / dir, filename)
+            return send_from_directory(directory, dir + os.path.sep + filename)
     return flask.Response("not found", status=404)
 
 
 @blueprint.route("/static/<path:path>")
 def serve_static(path):
@@ -215,11 +215,14 @@ if solara.settings.assets.proxy:
     @blueprint.route(f"/{cdn_helper.cdn_url_path}/<path:path>")
     def cdn(path):
         if not allowed():
             abort(401)
         cache_directory = settings.assets.proxy_cache_dir
-        content = cdn_helper.get_data(Path(cache_directory), path)
+        try:
+            content = cdn_helper.get_data(Path(cache_directory), path)
+        except PermissionError:
+            return flask.Response("not found", status=404)
         mime = mimetypes.guess_type(path)
         return flask.Response(content, mimetype=mime[0])
 
 
 @blueprint.route("/", defaults={"path": ""})
diff --git a/solara/server/starlette.py b/solara/server/starlette.py
index 299d3a9..a62c4a1 100644
--- a/solara/server/starlette.py
+++ b/solara/server/starlette.py
@@ -1,10 +1,11 @@
 import asyncio
 import json
 import logging
 import math
 import os
+from pathlib import Path
 import sys
 import threading
 import typing
 from typing import Any, Dict, List, Optional, Set, Union, cast
 from uuid import uuid4
@@ -12,10 +13,12 @@ from uuid import uuid4
 import anyio
 import starlette.websockets
 import uvicorn.server
 import websockets.legacy.http
 
+from solara.server.utils import path_is_child_of
+
 try:
     import solara_enterprise
 
     del solara_enterprise
     has_solara_enterprise = True
@@ -403,10 +406,13 @@ class StaticNbFiles(StaticFilesOptionalAuth):
     def lookup_path(self, path: str) -> typing.Tuple[str, typing.Optional[os.stat_result]]:
         for directory in self.all_directories:
             original_path = os.path.join(directory, path)
             full_path = os.path.realpath(original_path)
             directory = os.path.realpath(directory)
+            # return early if someone tries to access a file outside of the directory
+            if not path_is_child_of(Path(original_path), Path(directory)):
+                return "", None
             try:
                 return full_path, os.stat(full_path)
             except (FileNotFoundError, NotADirectoryError):
                 continue
         return "", None
@@ -447,11 +453,10 @@ class StaticCdn(StaticFilesOptionalAuth):
     def lookup_path(self, path: str) -> typing.Tuple[str, typing.Optional[os.stat_result]]:
         try:
             full_path = str(get_path(settings.assets.proxy_cache_dir, path))
         except Exception:
             return "", None
-
         return full_path, os.stat(full_path)
 
 
 def on_startup():
     # TODO: configure and set max number of threads
diff --git a/solara/server/utils.py b/solara/server/utils.py
index 42f1a3d..2f71894 100644
--- a/solara/server/utils.py
+++ b/solara/server/utils.py
@@ -1,8 +1,9 @@
 import contextlib
 import logging
 import os
+from pathlib import Path
 import pdb
 import traceback
 
 from rich import print
 
@@ -14,10 +15,16 @@ def start_error(title, msg, exception: Exception = None):
         traceback.print_exception(None, exception, exception.__traceback__)
     print(f"[red]{title}:\n\t[blue]{msg}")  # noqa
     os._exit(-1)
 
 
+def path_is_child_of(path: Path, parent: Path) -> bool:
+    # We use os.path.normpath() because we do not want to follow symlinks
+    # in editable installs, since some packages are symlinked
+    return os.path.normpath(path).startswith(os.path.normpath(parent))
+
+
 @contextlib.contextmanager
 def pdb_guard():
     from . import settings
 
     try:
diff --git a/tests/integration/apps/not-allowed b/tests/integration/apps/not-allowed
new file mode 100644
index 0000000..d3730bd
--- /dev/null
+++ b/tests/integration/apps/not-allowed
@@ -0,0 +1 @@
+not accessible
diff --git a/tests/integration/apps/public/test.txt b/tests/integration/apps/public/test.txt
new file mode 100644
index 0000000..9daeafb
--- /dev/null
+++ b/tests/integration/apps/public/test.txt
@@ -0,0 +1 @@
+test
diff --git a/tests/integration/apps/secure/app.py b/tests/integration/apps/secure/app.py
new file mode 100644
index 0000000..68986fc
--- /dev/null
+++ b/tests/integration/apps/secure/app.py
@@ -0,0 +1,4 @@
+import solara
+
+
+page = solara.Button("Click me")
diff --git a/tests/integration/server_test.py b/tests/integration/server_test.py
index 93a3320..5126f12 100644
--- a/tests/integration/server_test.py
+++ b/tests/integration/server_test.py
@@ -3,12 +3,15 @@ from pathlib import Path
 
 import playwright
 import playwright.sync_api
 import pytest
 import reacton.ipywidgets as w
+import requests
 
 import solara
+import solara.server.server
+from solara.server import settings
 
 HERE = Path(__file__).parent
 
 
 def test_large_cookie(browser: playwright.sync_api.Browser, page_session: playwright.sync_api.Page, solara_server, solara_app):
@@ -209,5 +212,102 @@ def test_kernel_asyncio(browser: playwright.sync_api.Browser, solara_server, sol
         page1.close()
         page2.close()
         context1.close()
         context2.close()
         solara.server.settings.kernel.threaded = threaded
+
+
+def test_cdn_secure(solara_server, solara_app, extra_include_path):
+    cdn_url = solara_server.base_url + "/_solara/cdn"
+    assert solara.settings.assets.proxy
+
+    with extra_include_path(HERE), solara_app("server_test:ClickButton"):
+        url = cdn_url + "/vue-grid-layout@1.0.2/dist/vue-grid-layout.min.js"
+        response = requests.get(url)
+        assert response.status_code == 200
+        # create a file in /share/solara
+        test_file = settings.assets.proxy_cache_dir.parent / "not-allowed"
+        test_file.write_text("test")
+        url = cdn_url + "/..%2fnot-allowed"
+        response = requests.get(url)
+        assert response.status_code == 404
+
+
+def test_nbextension_secure(solara_server, solara_app, extra_include_path):
+    nbextensions_url = solara_server.base_url + "/static/nbextensions"
+    nbextensions_directories = [k for k in solara.server.server.nbextensions_directories if k.exists()]
+    assert nbextensions_directories, "we should at least test one directory"
+    nbextensions_directory = nbextensions_directories[0]
+
+    with extra_include_path(HERE), solara_app("server_test:ClickButton"):
+        url = nbextensions_url + "/jupyter-vuetify/nodeps.js"
+        response = requests.get(url)
+        assert response.status_code == 200
+        test_file = nbextensions_directory.parent / "not-allowed"
+        test_file.write_text("test")
+        url = nbextensions_url + "/..%2fnot-allowed"
+        response = requests.get(url)
+        assert response.status_code == 404
+
+        url = nbextensions_url + "/foo/..%2f..%2fnot-allowed"
+        response = requests.get(url)
+        assert response.status_code == 404
+
+
+def test_assets_secure(solara_server, solara_app, extra_include_path):
+    assets_url = solara_server.base_url + "/static/assets"
+    assets_directory = solara.server.server.solara_static.parent / "assets"
+
+    with extra_include_path(HERE), solara_app("server_test:ClickButton"):
+        url = assets_url + "/theme.js"
+        response = requests.get(url)
+        assert response.status_code == 200
+        test_file = assets_directory.parent / "__init__.py"
+        assert test_file.exists()
+        url = assets_url + "/..%2f__init__.py"
+        response = requests.get(url)
+        assert response.status_code == 404
+
+        url = assets_url + "/foo/..%2f..%2f__init__.py"
+        response = requests.get(url)
+        assert response.status_code == 404
+
+
+def test_public_secure(solara_server, solara_app, extra_include_path):
+    public_url = solara_server.base_url + "/static/public"
+
+    with solara_app(str(HERE / "apps/secure/app.py")):
+        apps = list(solara.server.app.apps.values())
+        assert len(apps) == 1
+        app = apps[0]
+        public_directory = app.directory.parent / "public"
+        url = public_url + "/test.txt"
+        response = requests.get(url)
+        assert response.status_code == 200
+        test_file = public_directory.parent / "not-allowed"
+        assert test_file.exists()
+        url = public_url + "/..%2fnot-allowed"
+        response = requests.get(url)
+        assert response.status_code == 404
+
+        url = public_url + "/foo/..%2f..%2fnot-allowed"
+        response = requests.get(url)
+        assert response.status_code == 404
+
+
+def test_static_secure(solara_server, solara_app, extra_include_path):
+    static_url = solara_server.base_url + "/static"
+    static_directory = solara.server.server.solara_static
+
+    with extra_include_path(HERE), solara_app("server_test:ClickButton"):
+        url = static_url + "/main.js"
+        response = requests.get(url)
+        assert response.status_code == 200
+        test_file = static_directory.parent / "__init__.py"
+        assert test_file.exists()
+        url = static_url + "/..%2f__init__.py"
+        response = requests.get(url)
+        assert response.status_code == 404
+
+        url = static_url + "/foo/..%2f..%2f__init__.py"
+        response = requests.get(url)
+        assert response.status_code == 404

Vulnerable code: import os
    # We use os.path.normpath() because we do not want to follow symlinks
    # in editable installs, since some packages are symlinked
    if not os.path.normpath(cache_path).startswith(os.path.normpath(base_cache_dir)):
        logger.warning(
            "Trying to read from outside of cache directory: %s is not a subdir of %s (%s - %s)",
            cache_path,
            base_cache_dir,
            os.path.normpath(cache_path),
            os.path.normpath(base_cache_dir),
        )
            return send_from_directory(directory / dir, filename)
        content = cdn_helper.get_data(Path(cache_directory), path)

Fixed code: from solara.server.utils import path_is_child_of
    if not path_is_child_of(cache_path, base_cache_dir):
        raise PermissionError("Trying to write outside of cache directory")

    if not path_is_child_of(cache_path, base_cache_dir):
        logger.warning("Trying to read from outside of cache directory: %s is not a subdir of %s", cache_path, base_cache_dir)
    if not path_is_child_of(cache_path, base_cache_dir):
        raise PermissionError("Trying to read from outside of cache directory")

            return send_from_directory(directory, dir + os.path.sep + filename)
        try:
            content = cdn_helper.get_data(Path(cache_directory), path)
        except PermissionError:
            return flask.Response("not found", status=404)
from pathlib import Path
from solara.server.utils import path_is_child_of

            # return early if someone tries to access a file outside of the directory
            if not path_is_child_of(Path(original_path), Path(directory)):
                return "", None
from pathlib import Path
def path_is_child_of(path: Path, parent: Path) -> bool:
    # We use os.path.normpath() because we do not want to follow symlinks
    # in editable installs, since some packages are symlinked
    return os.path.normpath(path).startswith(os.path.normpath(parent))


not accessible
test
import solara


page = solara.Button("Click me")
import requests
import solara.server.server
from solara.server import settings


def test_cdn_secure(solara_server, solara_app, extra_include_path):
    cdn_url = solara_server.base_url + "/_solara/cdn"
    assert solara.settings.assets.proxy

    with extra_include_path(HERE), solara_app("server_test:ClickButton"):
        url = cdn_url + "/vue-grid-layout@1.0.2/dist/vue-grid-layout.min.js"
        response = requests.get(url)
        assert response.status_code == 200
        # create a file in /share/solara
        test_file = settings.assets.proxy_cache_dir.parent / "not-allowed"
        test_file.write_text("test")
        url = cdn_url + "/..%2fnot-allowed"
        response = requests.get(url)
        assert response.status_code == 404


def test_nbextension_secure(solara_server, solara_app, extra_include_path):
    nbextensions_url = solara_server.base_url + "/static/nbextensions"
    nbextensions_directories = [k for k in solara.server.server.nbextensions_directories if k.exists()]
    assert nbextensions_directories, "we should at least test one directory"
    nbextensions_directory = nbextensions_directories[0]

    with extra_include_path(HERE), solara_app("server_test:ClickButton"):
        url = nbextensions_url + "/jupyter-vuetify/nodeps.js"
        response = requests.get(url)
        assert response.status_code == 200
        test_file = nbextensions_directory.parent / "not-allowed"
        test_file.write_text("test")
        url = nbextensions_url + "/..%2fnot-allowed"
        response = requests.get(url)
        assert response.status_code == 404

        url = nbextensions_url + "/foo/..%2f..%2fnot-allowed"
        response = requests.get(url)
        assert response.status_code == 404


def test_assets_secure(solara_server, solara_app, extra_include_path):
    assets_url = solara_server.base_url + "/static/assets"
    assets_directory = solara.server.server.solara_static.parent / "assets"

    with extra_include_path(HERE), solara_app("server_test:ClickButton"):
        url = assets_url + "/theme.js"
        response = requests.get(url)
        assert response.status_code == 200
        test_file = assets_directory.parent / "__init__.py"
        assert test_file.exists()
        url = assets_url + "/..%2f__init__.py"
        response = requests.get(url)
        assert response.status_code == 404

        url = assets_url + "/foo/..%2f..%2f__init__.py"
        response = requests.get(url)
        assert response.status_code == 404


def test_public_secure(solara_server, solara_app, extra_include_path):
    public_url = solara_server.base_url + "/static/public"

    with solara_app(str(HERE / "apps/secure/app.py")):
        apps = list(solara.server.app.apps.values())
        assert len(apps) == 1
        app = apps[0]
        public_directory = app.directory.parent / "public"
        url = public_url + "/test.txt"
        response = requests.get(url)
        assert response.status_code == 200
        test_file = public_directory.parent / "not-allowed"
        assert test_file.exists()
        url = public_url + "/..%2fnot-allowed"
        response = requests.get(url)
        assert response.status_code == 404

        url = public_url + "/foo/..%2f..%2fnot-allowed"
        response = requests.get(url)
        assert response.status_code == 404


def test_static_secure(solara_server, solara_app, extra_include_path):
    static_url = solara_server.base_url + "/static"
    static_directory = solara.server.server.solara_static

    with extra_include_path(HERE), solara_app("server_test:ClickButton"):
        url = static_url + "/main.js"
        response = requests.get(url)
        assert response.status_code == 200
        test_file = static_directory.parent / "__init__.py"
        assert test_file.exists()
        url = static_url + "/..%2f__init__.py"
        response = requests.get(url)
        assert response.status_code == 404

        url = static_url + "/foo/..%2f..%2f__init__.py"
        response = requests.get(url)
        assert response.status_code == 404