Skip to content

Commit

Permalink
Merge pull request from GHSA-r25m-cr6v-p9hq
Browse files Browse the repository at this point in the history
* fix for vuln

* Apply suggestions from code review

Use correct spelling of malicious.

Co-authored-by: Adam Sachs <adam@ethyca.com>

* Update src/fides/api/main.py

Verbiage

Co-authored-by: Thomas <thomas.lapiana+github@pm.me>

---------

Co-authored-by: Adam Sachs <adam@ethyca.com>
Co-authored-by: Thomas <thomas.lapiana+github@pm.me>
Co-authored-by: Dave Quinlan <83430497+daveqnet@users.noreply.github.com>
  • Loading branch information
4 people committed Jul 5, 2023
1 parent 1212ead commit f526d9f
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 0 deletions.
4 changes: 4 additions & 0 deletions src/fides/api/common_exceptions.py
Expand Up @@ -223,6 +223,10 @@ class SSHTunnelConfigNotFoundException(Exception):
"""Exception for when Fides is configured to use an SSH tunnel without config provided."""


class MalisciousUrlException(Exception):
"""Fides has detected a potentially maliscious URL."""


class AuthenticationError(HTTPException):
"""To be raised when attempting to fetch an access token using
invalid credentials.
Expand Down
20 changes: 20 additions & 0 deletions src/fides/api/main.py
@@ -1,6 +1,7 @@
"""
Contains the code that sets up the API.
"""
import os
import sys
from datetime import datetime, timezone
from logging import WARNING
Expand All @@ -11,6 +12,7 @@
from fideslog.sdk.python.event import AnalyticsEvent
from loguru import logger
from starlette.background import BackgroundTask
from urllib.parse import unquote
from uvicorn import Config, Server

import fides
Expand All @@ -20,6 +22,7 @@
log_startup,
run_database_startup,
)
from fides.api.common_exceptions import MalisciousUrlException
from fides.api.middleware import handle_audit_log_resource
from fides.api.schemas.analytics import Event, ExtraData

Expand Down Expand Up @@ -151,13 +154,30 @@ def read_index() -> Response:
return get_admin_index_as_response()


def sanitise_url_path(path: str) -> str:
"""Returns a URL path that does not contain any ../ or //"""
path = unquote(path)
path = os.path.normpath(path)
for token in path.split("/"):
if ".." in token:
logger.warning(f"Potentially dangerous use of URL hierarchy in path: {path}")
raise MalisciousUrlException()
return path


@app.get("/{catchall:path}", response_class=Response, tags=["Default"])
def read_other_paths(request: Request) -> Response:
"""
Return related frontend files. Adapted from https://github.com/tiangolo/fastapi/issues/130
"""
# check first if requested file exists (for frontend assets)
path = request.path_params["catchall"]
logger.debug(f"Catch all path detected: {path}")
try:
path = sanitise_url_path(path)
except MalisciousUrlException:
# if a maliscious URL is detected, route the user to the index
return get_admin_index_as_response()

# search for matching route in package (i.e. /dataset)
ui_file = match_route(get_ui_file_map(), path)
Expand Down
21 changes: 21 additions & 0 deletions tests/ops/util/test_api_router.py
Expand Up @@ -45,3 +45,24 @@ def test_non_existent_route_404(
f"{V1_URL_PREFIX}/route/does/not/exist/", headers=auth_header
)
assert resp_4.status_code == HTTP_404_NOT_FOUND

def test_malicious_url(
self,
api_client: TestClient,
url,
) -> None:
malicious_paths = [
"../../../../../../../../../etc/passwd",
"..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2f..%2fetc/passwd",
"%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/%2e%2e/etc/passwd",
"%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2f%2e%2e%2fetc/passwd",
"..%c0%2f..%c0%2f..%c0%2f..%c0%2f..%c0%2f..%c0%2f..%c0%2f..%c0%2f..%c0%2f/etc/passwd",
".../...//.../...//.../...//.../...//.../...//.../...//.../...//.../...//.../...//etc/passwd",
"...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2f...%2f...%2f%2fetc/passwd",
"%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//%2e%2e%2e/%2e%2e%2e//etc/passwd",
"%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2f%2e%2e%2e%2f%2e%2e%2e%2f%2fetc/passwd",
]
for path in malicious_paths:
resp = api_client.get(f"{url}/{path}")
assert resp.status_code == 200
assert resp.text == "<h1>Privacy is a Human Right!</h1>"

0 comments on commit f526d9f

Please sign in to comment.