CVE-2020-15239: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: In xmpp-http-upload before version 0.4.0, when the GET method is attacked, attackers can read files which have a `.data` suffix and which are accompanied by a JSON file with the `.meta` suffix. This can lead to Information Disclosure and in some shared-hosting scenarios also to circumvention of authentication or other limitations on the outbound (GET) traffic. For example, in a scenario where a single server has multiple instances of the application running (with separate DATA_ROOT settings), an attacker who has knowledge about the directory structure is able to read files from any other instance to which the process has read access. If instances have individual authentication (for example, HTTP authentication via a reverse proxy, source IP based filtering) or other restrictions (such as quotas), attackers may circumvent those limits in such a scenario by using the Directory Traversal to retrieve data from the other instances. If the associated XMPP server (or anyone knowing the SECRET_KEY) is malicious, they can write files outside the DATA_ROOT. The files which are written are constrained to have the `.meta` and the `.data` suffixes; the `.meta` file will contain the JSON with the Content-Type of the original request and the `.data` file will contain the payload. The issue is patched in version 0.4.0.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
CVSS v3 base score: 3.5
Published: 2020-10-06T19:15Z
Repository: https://github.com/horazont/xmpp-http-upload
Language: Python
Files changed: xhu.py
Commit date: 2020-10-06 01:06:21 +0200
Commit message: Simplify path handling, use safe_join

The current implementation of sanitized_join did not handle
".." properly. The problem is, that .absolute() does not do
what .resolve() does, but .resolve() does not work on non
existant paths.

Anyway, flask has a function exactly for this: safe_join.

So let's use that one.

While at it, simplified the whole path handling a bit.

Diff: diff --git a/xhu.py b/xhu.py
index a28c5a8..b999957 100644
--- a/xhu.py
+++ b/xhu.py
@@ -27,48 +27,39 @@ import hashlib
 import hmac
 import pathlib
 import typing
 
 import flask
+import werkzeug.exceptions
 
 app = flask.Flask("xmpp-http-upload")
 app.config.from_envvar("XMPP_HTTP_UPLOAD_CONFIG")
 application = app
 
 if app.config['ENABLE_CORS']:
     from flask_cors import CORS
     CORS(app)
 
 
-def sanitized_join(path: str, root: pathlib.Path) -> pathlib.Path:
-    result = (root / path).absolute()
-    if not str(result).startswith(str(root) + "/"):
-        raise ValueError("resulting path is outside root")
-    return result
-
-
-def get_paths(base_path: pathlib.Path):
-    data_file = pathlib.Path(str(base_path) + ".data")
-    metadata_file = pathlib.Path(str(base_path) + ".meta")
+def get_paths(root: str, sub_path: str) \
+        -> typing.Tuple[pathlib.Path, pathlib.Path]:
+    base_path = flask.safe_join(root, sub_path)
+    data_file = pathlib.Path(base_path + ".data")
+    metadata_file = pathlib.Path(base_path + ".meta")
 
     return data_file, metadata_file
 
 
 def load_metadata(metadata_file):
     with metadata_file.open("r") as f:
         return json.load(f)
 
 
-def get_info(path: str, root: pathlib.Path) -> typing.Tuple[
+def get_info(path: str) -> typing.Tuple[
         pathlib.Path,
         dict]:
-    dest_path = sanitized_join(
-        path,
-        pathlib.Path(app.config["DATA_ROOT"]),
-    )
-
-    data_file, metadata_file = get_paths(dest_path)
+    data_file, metadata_file = get_paths(app.config["DATA_ROOT"], path)
 
     return data_file, load_metadata(metadata_file)
 
 
 @contextlib.contextmanager
@@ -102,15 +93,12 @@ def stream_file(src, dest, nbytes):
 
 
 @app.route("/<path:path>", methods=["PUT"])
 def put_file(path):
     try:
-        dest_path = sanitized_join(
-            path,
-            pathlib.Path(app.config["DATA_ROOT"]),
-        )
-    except ValueError:
+        data_file, metadata_file = get_paths(app.config["DATA_ROOT"], path)
+    except werkzeug.exceptions.NotFound:
         return flask.Response(
             "Not Found",
             404,
             mimetype="text/plain",
         )
@@ -132,12 +120,11 @@ def put_file(path):
     content_type = flask.request.headers.get(
         "Content-Type",
         "application/octet-stream",
     )
 
-    dest_path.parent.mkdir(parents=True, exist_ok=True, mode=0o770)
-    data_file, metadata_file = get_paths(dest_path)
+    data_file.parent.mkdir(parents=True, exist_ok=True, mode=0o770)
 
     try:
         with write_file(data_file) as fout:
             stream_file(flask.request.stream, fout, length)
 
@@ -187,17 +174,14 @@ def generate_headers(response_headers, metadata_headers):
 
 
 @app.route("/<path:path>", methods=["HEAD"])
 def head_file(path):
     try:
-        data_file, metadata = get_info(
-            path,
-            pathlib.Path(app.config["DATA_ROOT"])
-        )
+        data_file, metadata = get_info(path)
 
         stat = data_file.stat()
-    except (OSError, ValueError):
+    except (OSError, werkzeug.exceptions.NotFound):
         return flask.Response(
             "Not Found",
             404,
             mimetype="text/plain",
         )
@@ -212,15 +196,12 @@ def head_file(path):
 
 
 @app.route("/<path:path>", methods=["GET"])
 def get_file(path):
     try:
-        data_file, metadata = get_info(
-            path,
-            pathlib.Path(app.config["DATA_ROOT"])
-        )
-    except (OSError, ValueError):
+        data_file, metadata = get_info(path)
+    except (OSError, werkzeug.exceptions.NotFound):
         return flask.Response(
             "Not Found",
             404,
             mimetype="text/plain",
         )

Vulnerable code: def sanitized_join(path: str, root: pathlib.Path) -> pathlib.Path:
    result = (root / path).absolute()
    if not str(result).startswith(str(root) + "/"):
        raise ValueError("resulting path is outside root")
    return result


def get_paths(base_path: pathlib.Path):
    data_file = pathlib.Path(str(base_path) + ".data")
    metadata_file = pathlib.Path(str(base_path) + ".meta")
def get_info(path: str, root: pathlib.Path) -> typing.Tuple[
    dest_path = sanitized_join(
        path,
        pathlib.Path(app.config["DATA_ROOT"]),
    )

    data_file, metadata_file = get_paths(dest_path)
        dest_path = sanitized_join(
            path,
            pathlib.Path(app.config["DATA_ROOT"]),
        )
    except ValueError:
    dest_path.parent.mkdir(parents=True, exist_ok=True, mode=0o770)
    data_file, metadata_file = get_paths(dest_path)
        data_file, metadata = get_info(
            path,
            pathlib.Path(app.config["DATA_ROOT"])
        )
    except (OSError, ValueError):
        data_file, metadata = get_info(
            path,
            pathlib.Path(app.config["DATA_ROOT"])
        )
    except (OSError, ValueError):
Fixed code: import werkzeug.exceptions
def get_paths(root: str, sub_path: str) \
        -> typing.Tuple[pathlib.Path, pathlib.Path]:
    base_path = flask.safe_join(root, sub_path)
    data_file = pathlib.Path(base_path + ".data")
    metadata_file = pathlib.Path(base_path + ".meta")
def get_info(path: str) -> typing.Tuple[
    data_file, metadata_file = get_paths(app.config["DATA_ROOT"], path)
        data_file, metadata_file = get_paths(app.config["DATA_ROOT"], path)
    except werkzeug.exceptions.NotFound:
    data_file.parent.mkdir(parents=True, exist_ok=True, mode=0o770)
        data_file, metadata = get_info(path)
    except (OSError, werkzeug.exceptions.NotFound):
        data_file, metadata = get_info(path)
    except (OSError, werkzeug.exceptions.NotFound):