Skip to content

Conversation

d3do-23
Copy link

@d3do-23 d3do-23 commented Jul 6, 2025

Description

This PR addresses two critical security vulnerabilities found in the media content processing logic within the _process_request function:

  1. Local File Inclusion (LFI): The code did not properly sanitize user-provided file paths when handling image_url, video_url, and audio_url. An attacker could use path traversal payloads (e.g., ../../etc/passwd) to read arbitrary files from the server's filesystem.

  2. Server-Side Request Forgery (SSRF): The code directly fetched any user-provided URL using requests.get. An attacker could abuse this to make the server send requests to internal network services, cloud metadata endpoints (e.g., 169.254.169.254), or other sensitive resources.

Implementation Details

To fix these issues, this PR introduces a non-invasive, targeted approach by adding two new security check functions:

  • _check_lfi_path(path): This function is called right before open(). It validates that the resolved absolute path of a local file is within a pre-configured safe directory (SAFE_MEDIA_PATH), effectively preventing any directory traversal.

  • _check_ssrf_url(url): This function is called right before requests.get(). It provides two layers of defense against Server-Side Request Forgery:

    • URL Whitelist: It first checks the URL against a configurable list of allowed prefixes (ALLOWED_URL_PREFIXES). If this whitelist is active, the URL must start with one of the approved prefixes to proceed.
    • IP Address Validation: It then resolves the URL's hostname to an IP address and verifies that it is a public IP (is_global). This crucial check is always performed, even for whitelisted URLs, to protect against attacks like DNS rebinding. This prevents the server from making requests to private, loopback, or reserved IP ranges.

These checks are inserted into the existing logic without significant refactoring, preserving the original code structure.

Configuration

The fix also introduces two environment variables for better control and flexibility:

  • SAFE_MEDIA_PATH: Defines the secure directory from which local files can be served.
  • ALLOW_LOCAL_FILES: Acts as a feature flag to enable or disable the local file loading functionality entirely.

This ensures the application is secure by default while allowing administrators to configure it as needed.

@d3do-23
Copy link
Author

d3do-23 commented Jul 9, 2025

@hiyouga Is there anything that needs to be changed?

@d3do-23 d3do-23 changed the title Security: Fix LFI and SSRF Vulnerabilities in Media URL Handling fix some features Jul 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants