CodeQL: How to resolve partial Server-Side Request Forgery warning when taking user-based input as a FastAPI endpoint?

Question

cross-posting here on Information Security as well to request for input on the security of the Python code I've written, and whether it can be considered a false positive.

I'm writing a function to return a file from an MSYS2 package repo, and have been repeatedly encountering this SSRF warning from CodeQL, saying that I haven't done enough to validate my user-based inputs. The function I'm writing is listed below:

import re
from urllib.parse import quote

import requests
from fastapi import APIRouter, HTTPException, Response


# Set up FastAPI router
msys2 = APIRouter(prefix="/msys2")

# List of valid inputs, used over multiple endpoints
valid_env = ("msys", "mingw")
valid_msys = ("i686", "x86_64")
valid_mingw = (
    "clang32",
    "clang64",
    "clangarm64",
    "i686",
    "mingw32",
    "mingw64",
    "sources",
    "ucrt64",
    "x86_64",
)


@msys2.get("/{environment}/{architecture}/{package}", response_class=Response)
def get_msys2_package_file(
    environment: str,
    architecture: str,
    package: str,
) -> Response:
    """
    Obtain and pass through a specific download for an MSYS2 package.
    """

    # Validate environment
    if environment not in valid_env:
        raise ValueError(f"{environment!r} is not a valid msys2 environment")

    # Validate architecture for each environment
    if environment == "msys" and architecture not in valid_msys:
        raise ValueError(f"{architecture!r} is not a valid msys architecture")
    elif environment == "mingw" and architecture not in valid_mingw:
        raise ValueError(f"{architecture!r} is not a valid mingw architecture")

    # Validate package name
    if bool(re.fullmatch(r"^[\w\s\.\-]+$", package)) is False:
        raise ValueError(f"{package!r} is not a valid package name")

    # Construct URL to main MSYS repo and get response
    package_url = f"https://repo.msys2.org/{quote(environment)}/{quote(architecture)}/{quote(package)}"
    package_file = requests.get(package_url)

    if package_file.status_code == 200:
        return Response(
            content=package_file.content,
            media_type=package_file.headers.get("Content-Type"),
            status_code=package_file.status_code,
        )
    else:
        raise HTTPException(status_code=package_file.status_code)

Should this be considered a false positive, or are there ways I can further validate and improve the security of this function? Thanks!

Answer 1

It's a false positive. All parameters are properly validated and escaped. You could add some redundant regex checks to make CodeQL happy, but this would be a waste of time. The point of using a scanner is to fix actual vulnerabilities, not to spend your time coming up with workarounds for the scanner's limitations.

A few parts of the code aren't optimal, though:

• The validation of the architecture parameter is fragile, because it's only triggered if environment is either msys or mingw. This works fine in the current code, because environment is currently restricted to valid_env, and this array happens to contain exactly those two strings. However, should you ever change valid_env, then you may suddenly end up without any validation of architecture at all. To fix this, add an else case raising an error like “No valid architectures defined for environment ...”
• The function urllib.parse.quote doesn't quote forward slashes by default, which means you could end up “injecting” those into the path. To fix this, set the safe parameter of urllib.parse.quote to None or the empty string.