-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Fix: prevent yt-dlp argument injection via unsanitized URL (RCE) #58
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -4,6 +4,7 @@ | |
| import json | ||
| import subprocess | ||
| import threading | ||
| from urllib.parse import urlparse | ||
| from flask import Flask, request, jsonify, send_file, render_template | ||
|
|
||
| app = Flask(__name__) | ||
|
|
@@ -13,6 +14,20 @@ | |
| jobs = {} | ||
|
|
||
|
|
||
| def is_safe_url(url): | ||
| """Reject anything that isn't a plain http(s) URL. | ||
|
|
||
| This also blocks strings starting with ``-``/``--`` which yt-dlp would | ||
| otherwise parse as CLI options (e.g. ``--exec``), letting a caller | ||
| smuggle arbitrary flags into the subprocess invocation. | ||
| """ | ||
| try: | ||
| parsed = urlparse(url) | ||
| except ValueError: | ||
| return False | ||
| return parsed.scheme in ("http", "https") and bool(parsed.netloc) | ||
|
|
||
|
|
||
| def parse_ytdlp_json(stdout): | ||
| """Parse yt-dlp JSON output. | ||
|
|
||
|
|
@@ -42,7 +57,10 @@ def run_download(job_id, url, format_choice, format_id): | |
| else: | ||
| cmd += ["-f", "bestvideo+bestaudio/best", "--merge-output-format", "mp4"] | ||
|
|
||
| cmd.append(url) | ||
| # "--" stops yt-dlp from treating a URL that begins with "-" as an | ||
| # option (e.g. "--exec=..."), which would otherwise allow arbitrary | ||
| # command execution. | ||
| cmd += ["--", url] | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The -- separator here is what closes the injection - verified on yt-dlp 2026.08.19 that a leading '-' value is treated as a URL (not an option) once it precedes it. Consider hardening is_safe_url against RFC-1918/loopback hosts and credential-in-URL (http://user:pass@host) to block SSRF/credential smuggling in future endpoints. |
||
|
|
||
| try: | ||
| result = subprocess.run(cmd, capture_output=True, text=True, timeout=300) | ||
|
|
@@ -100,8 +118,10 @@ def get_info(): | |
| url = data.get("url", "").strip() | ||
| if not url: | ||
| return jsonify({"error": "No URL provided"}), 400 | ||
| if not is_safe_url(url): | ||
| return jsonify({"error": "Invalid URL"}), 400 | ||
|
|
||
| cmd = ["yt-dlp", "--no-playlist", "-j", url] | ||
| cmd = ["yt-dlp", "--no-playlist", "-j", "--", url] | ||
| try: | ||
| result = subprocess.run(cmd, capture_output=True, text=True, timeout=60) | ||
| if result.returncode != 0: | ||
|
|
@@ -146,8 +166,10 @@ def get_playlist_info(): | |
| url = data.get("url", "").strip() | ||
| if not url: | ||
| return jsonify({"error": "No URL provided"}), 400 | ||
| if not is_safe_url(url): | ||
| return jsonify({"error": "Invalid URL"}), 400 | ||
|
|
||
| cmd = ["yt-dlp", "--flat-playlist", "-J", url] | ||
| cmd = ["yt-dlp", "--flat-playlist", "-J", "--", url] | ||
| try: | ||
| result = subprocess.run(cmd, capture_output=True, text=True, timeout=60) | ||
| if result.returncode != 0: | ||
|
|
@@ -173,6 +195,8 @@ def start_download(): | |
|
|
||
| if not url: | ||
| return jsonify({"error": "No URL provided"}), 400 | ||
| if not is_safe_url(url): | ||
| return jsonify({"error": "Invalid URL"}), 400 | ||
|
|
||
| job_id = uuid.uuid4().hex[:10] | ||
| jobs[job_id] = {"status": "downloading", "url": url, "title": title} | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
--separator here is what closes the injection — I verified on yt-dlp 2026.08.19 that a leading '-' value is treated as a URL (not an option) once '--' precedes it. Consider hardeningis_safe_urlagainst RFC-1918/loopback hosts (and credential-in-URL likehttp://user:pass@host) to block SSRF/credential smuggling in future endpoints.