CVE-2024-22423: Insufficient Information (NVD-CWE-noinfo)
value: yt-dlp is a youtube-dl fork with additional features and fixes. The patch that addressed CVE-2023-40581 attempted to prevent RCE when using `--exec` with `%q` by replacing double quotes with two double quotes. However, this escaping is not sufficient, and still allows expansion of environment variables. Support for output template expansion in `--exec`, along with this vulnerable behavior, was added to `yt-dlp` in version 2021.04.11. yt-dlp version 2024.04.09 fixes this issue by properly escaping `%`. It replaces them with `%%cd:~,%`, a variable that expands to nothing, leaving only the leading percent. It is recommended to upgrade yt-dlp to version 2024.04.09 as soon as possible. Also, always be careful when using `--exec`, because while this specific vulnerability has been patched, using unvalidated input in shell commands is inherently dangerous. For Windows users who are not able to upgrade, avoid using any output template expansion in `--exec` other than `{}` (filepath); if expansion in `--exec` is needed, verify the fields you are using do not contain `"`, `|` or `&`; and/or instead of using `--exec`, write the info json and load the fields from it instead.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-04-09T18:15Z
Repository: https://github.com/yt-dlp/yt-dlp
Language: Python
Files changed: devscripts/changelog_override.json, test/test_utils.py, yt_dlp/YoutubeDL.py, yt_dlp/compat/__init__.py, yt_dlp/utils/_utils.py
Commit date: 2024-04-09 18:36:13 +0200
Commit message: [core] Prevent RCE when using `--exec` with `%q` (CVE-2024-22423)

The shell escape function now properly escapes `%`, `\\` and `\n`. `utils.Popen` as well as `%q` output template expansion have been patched accordingly.

Prior to this fix using `--exec` together with `%q` when on Windows could cause remote code to execute. See https://github.com/yt-dlp/yt-dlp/security/advisories/GHSA-hjq6-52gw-2g7p for more details.

Authored by: Grub4K

Diff: diff --git a/devscripts/changelog_override.json b/devscripts/changelog_override.json
index 52ddf0613d..046060cb25 100644
--- a/devscripts/changelog_override.json
+++ b/devscripts/changelog_override.json
@@ -140,7 +140,12 @@
     {
         "action": "change",
         "when": "e3a3ed8a981d9395c4859b6ef56cd02bc3148db2",
         "short": "[cleanup:ie] No `from` stdlib imports in extractors",
         "authors": ["pukkandan"]
+    },
+    {
+        "action": "add",
+        "when": "9590cc6b4768e190183d7d071a6c78170889116a",
+        "short": "[priority] Security: [[CVE-2024-22423](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-22423)] [Prevent RCE when using `--exec` with `%q` on Windows](https://github.com/yt-dlp/yt-dlp/security/advisories/GHSA-hjq6-52gw-2g7p)\n    - The shell escape function now properly escapes `%`, `\\` and `\\n`.\n    - `utils.Popen` has been patched accordingly."
     }
 ]
diff --git a/test/test_utils.py b/test/test_utils.py
index 71febeefd6..ddf0a7c242 100644
--- a/test/test_utils.py
+++ b/test/test_utils.py
@@ -2067,10 +2067,14 @@ def run_shell(args):
             assert not error
             return stdout
 
         # Test escaping
         assert run_shell(['echo', 'test"&']) == '"test""&"\n'
+        assert run_shell(['echo', '%CMDCMDLINE:~-1%&']) == '"%CMDCMDLINE:~-1%&"\n'
+        assert run_shell(['echo', 'a\nb']) == '"a"\n"b"\n'
+        assert run_shell(['echo', '"']) == '""""\n'
+        assert run_shell(['echo', '\\']) == '\\\n'
         # Test if delayed expansion is disabled
         assert run_shell(['echo', '^!']) == '"^!"\n'
         assert run_shell('echo "^!"') == '"^!"\n'
 
 
diff --git a/yt_dlp/YoutubeDL.py b/yt_dlp/YoutubeDL.py
index 35aba968fb..9f730d0384 100644
--- a/yt_dlp/YoutubeDL.py
+++ b/yt_dlp/YoutubeDL.py
@@ -23,11 +23,11 @@
 import traceback
 import unicodedata
 
 from .cache import Cache
 from .compat import functools, urllib  # isort: split
-from .compat import compat_os_name, compat_shlex_quote, urllib_req_to_req
+from .compat import compat_os_name, urllib_req_to_req
 from .cookies import LenientSimpleCookie, load_cookies
 from .downloader import FFmpegFD, get_suitable_downloader, shorten_protocol_name
 from .downloader.rtmp import rtmpdump_version
 from .extractor import gen_extractor_classes, get_info_extractor
 from .extractor.common import UnsupportedURLIE
@@ -100,11 +100,10 @@
     SameFileError,
     UnavailableVideoError,
     UserNotLive,
     YoutubeDLError,
     age_restricted,
-    args_to_str,
     bug_reports_message,
     date_from_str,
     deprecation_warning,
     determine_ext,
     determine_protocol,
@@ -139,10 +138,11 @@
     render_table,
     replace_extension,
     sanitize_filename,
     sanitize_path,
     sanitize_url,
+    shell_quote,
     str_or_none,
     strftime_or_none,
     subtitles_filename,
     supports_terminal_sequences,
     system_identifier,
@@ -821,11 +821,11 @@ def warn_if_short_id(self, argv):
                 + ['--'] + [argv[i] for i in idxs]
             )
             self.report_warning(
                 'Long argument string detected. '
                 'Use -- to separate parameters and URLs, like this:\n%s' %
-                args_to_str(correct_argv))
+                shell_quote(correct_argv))
 
     def add_info_extractor(self, ie):
         """Add an InfoExtractor object to the end of the list."""
         ie_key = ie.ie_key()
         self._ies[ie_key] = ie
@@ -1353,11 +1353,11 @@ def create_key(outer_mobj):
                     indent=4 if '#' in flags else None, ensure_ascii='+' not in flags), str_fmt
             elif fmt[-1] == 'h':  # html
                 value, fmt = escapeHTML(str(value)), str_fmt
             elif fmt[-1] == 'q':  # quoted
                 value = map(str, variadic(value) if '#' in flags else [value])
-                value, fmt = ' '.join(map(compat_shlex_quote, value)), str_fmt
+                value, fmt = shell_quote(value, shell=True), str_fmt
             elif fmt[-1] == 'B':  # bytes
                 value = f'%{str_fmt}'.encode() % str(value).encode()
                 value, fmt = value.decode('utf-8', 'ignore'), 's'
             elif fmt[-1] == 'U':  # unicode normalized
                 value, fmt = unicodedata.normalize(
diff --git a/yt_dlp/compat/__init__.py b/yt_dlp/compat/__init__.py
index 5ad5c70ecf..d820adaf1e 100644
--- a/yt_dlp/compat/__init__.py
+++ b/yt_dlp/compat/__init__.py
@@ -25,16 +25,13 @@ def compat_etree_fromstring(text):
 
 
 compat_os_name = os._name if os.name == 'java' else os.name
 
 
-if compat_os_name == 'nt':
-    def compat_shlex_quote(s):
-        import re
-        return s if re.match(r'^[-_\w./]+$', s) else s.replace('"', '""').join('""')
-else:
-    from shlex import quote as compat_shlex_quote  # noqa: F401
+def compat_shlex_quote(s):
+    from ..utils import shell_quote
+    return shell_quote(s)
 
 
 def compat_ord(c):
     return c if isinstance(c, int) else ord(c)
 
diff --git a/yt_dlp/utils/_utils.py b/yt_dlp/utils/_utils.py
index dec514674f..e3e80f3d33 100644
--- a/yt_dlp/utils/_utils.py
+++ b/yt_dlp/utils/_utils.py
@@ -48,11 +48,10 @@
 from ..compat import (
     compat_etree_fromstring,
     compat_expanduser,
     compat_HTMLParseError,
     compat_os_name,
-    compat_shlex_quote,
 )
 from ..dependencies import xattr
 
 __name__ = __name__.rsplit('.', 1)[0]  # Pretend to be the parent module
 
@@ -834,13 +833,15 @@ def __init__(self, args, *remaining, env=None, text=False, shell=False, **kwargs
             kwargs.setdefault('encoding', 'utf-8')
             kwargs.setdefault('errors', 'replace')
 
         if shell and compat_os_name == 'nt' and kwargs.get('executable') is None:
             if not isinstance(args, str):
-                args = ' '.join(compat_shlex_quote(a) for a in args)
+                args = shell_quote(args, shell=True)
             shell = False
-            args = f'{self.__comspec()} /Q /S /D /V:OFF /C "{args}"'
+            # Set variable for `cmd.exe` newline escaping (see `utils.shell_quote`)
+            env['='] = '"^\n\n"'
+            args = f'{self.__comspec()} /Q /S /D /V:OFF /E:ON /C "{args}"'
 
         super().__init__(args, *remaining, env=env, shell=shell, **kwargs, startupinfo=self._startupinfo)
 
     def __comspec(self):
         comspec = os.environ.get('ComSpec') or os.path.join(
@@ -1635,19 +1636,42 @@ def __iter__(self):
 def get_filesystem_encoding():
     encoding = sys.getfilesystemencoding()
     return encoding if encoding is not None else 'utf-8'
 
 
-def shell_quote(args):
-    quoted_args = []
-    encoding = get_filesystem_encoding()
-    for a in args:
-        if isinstance(a, bytes):
-            # We may get a filename encoded with 'encodeFilename'
-            a = a.decode(encoding)
-        quoted_args.append(compat_shlex_quote(a))
-    return ' '.join(quoted_args)
+_WINDOWS_QUOTE_TRANS = str.maketrans({'"': '\\"', '\\': '\\\\'})
+_CMD_QUOTE_TRANS = str.maketrans({
+    # Keep quotes balanced by replacing them with `""` instead of `\\"`
+    '"': '""',
+    # Requires a variable `=` containing `"^\n\n"` (set in `utils.Popen`)
+    # `=` should be unique since variables containing `=` cannot be set using cmd
+    '\n': '%=%',
+    # While we are only required to escape backslashes immediately before quotes,
+    # we instead escape all of 'em anyways to be consistent
+    '\\': '\\\\',
+    # Use zero length variable replacement so `%` doesn't get expanded
+    # `cd` is always set as long as extensions are enabled (`/E:ON` in `utils.Popen`)
+    '%': '%%cd:~,%',
+})
+
+
+def shell_quote(args, *, shell=False):
+    args = list(variadic(args))
+    if any(isinstance(item, bytes) for item in args):
+        deprecation_warning('Passing bytes to utils.shell_quote is deprecated')
+        encoding = get_filesystem_encoding()
+        for index, item in enumerate(args):
+            if isinstance(item, bytes):
+                args[index] = item.decode(encoding)
+
+    if compat_os_name != 'nt':
+        return shlex.join(args)
+
+    trans = _CMD_QUOTE_TRANS if shell else _WINDOWS_QUOTE_TRANS
+    return ' '.join(
+        s if re.fullmatch(r'[\w#$*\-+./:?@\\]+', s, re.ASCII) else s.translate(trans).join('""')
+        for s in args)
 
 
 def smuggle_url(url, data):
     """ Pass additional data in a URL for internal use. """
 
@@ -2847,11 +2871,11 @@ def ytdl_is_updateable():
     return not is_non_updateable()
 
 
 def args_to_str(args):
     # Get a short string representation for a subprocess command
-    return ' '.join(compat_shlex_quote(a) for a in args)
+    return shell_quote(args)
 
 
 def error_to_str(err):
     return f'{type(err).__name__}: {err}'
 

Vulnerable code: from .compat import compat_os_name, compat_shlex_quote, urllib_req_to_req
    args_to_str,
                args_to_str(correct_argv))
                value, fmt = ' '.join(map(compat_shlex_quote, value)), str_fmt
if compat_os_name == 'nt':
    def compat_shlex_quote(s):
        import re
        return s if re.match(r'^[-_\w./]+$', s) else s.replace('"', '""').join('""')
else:
    from shlex import quote as compat_shlex_quote  # noqa: F401
    compat_shlex_quote,
                args = ' '.join(compat_shlex_quote(a) for a in args)
            args = f'{self.__comspec()} /Q /S /D /V:OFF /C "{args}"'
def shell_quote(args):
    quoted_args = []
    encoding = get_filesystem_encoding()
    for a in args:
        if isinstance(a, bytes):
            # We may get a filename encoded with 'encodeFilename'
            a = a.decode(encoding)
        quoted_args.append(compat_shlex_quote(a))
    return ' '.join(quoted_args)
    return ' '.join(compat_shlex_quote(a) for a in args)
Fixed code:     },
    {
        "action": "add",
        "when": "9590cc6b4768e190183d7d071a6c78170889116a",
        "short": "[priority] Security: [[CVE-2024-22423](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2024-22423)] [Prevent RCE when using `--exec` with `%q` on Windows](https://github.com/yt-dlp/yt-dlp/security/advisories/GHSA-hjq6-52gw-2g7p)\n    - The shell escape function now properly escapes `%`, `\\` and `\\n`.\n    - `utils.Popen` has been patched accordingly."
        assert run_shell(['echo', '%CMDCMDLINE:~-1%&']) == '"%CMDCMDLINE:~-1%&"\n'
        assert run_shell(['echo', 'a\nb']) == '"a"\n"b"\n'
        assert run_shell(['echo', '"']) == '""""\n'
        assert run_shell(['echo', '\\']) == '\\\n'
from .compat import compat_os_name, urllib_req_to_req
    shell_quote,
                shell_quote(correct_argv))
                value, fmt = shell_quote(value, shell=True), str_fmt
def compat_shlex_quote(s):
    from ..utils import shell_quote
    return shell_quote(s)
                args = shell_quote(args, shell=True)
            # Set variable for `cmd.exe` newline escaping (see `utils.shell_quote`)
            env['='] = '"^\n\n"'
            args = f'{self.__comspec()} /Q /S /D /V:OFF /E:ON /C "{args}"'
_WINDOWS_QUOTE_TRANS = str.maketrans({'"': '\\"', '\\': '\\\\'})
_CMD_QUOTE_TRANS = str.maketrans({
    # Keep quotes balanced by replacing them with `""` instead of `\\"`
    '"': '""',
    # Requires a variable `=` containing `"^\n\n"` (set in `utils.Popen`)
    # `=` should be unique since variables containing `=` cannot be set using cmd
    '\n': '%=%',
    # While we are only required to escape backslashes immediately before quotes,
    # we instead escape all of 'em anyways to be consistent
    '\\': '\\\\',
    # Use zero length variable replacement so `%` doesn't get expanded
    # `cd` is always set as long as extensions are enabled (`/E:ON` in `utils.Popen`)
    '%': '%%cd:~,%',
})


def shell_quote(args, *, shell=False):
    args = list(variadic(args))
    if any(isinstance(item, bytes) for item in args):
        deprecation_warning('Passing bytes to utils.shell_quote is deprecated')
        encoding = get_filesystem_encoding()
        for index, item in enumerate(args):
            if isinstance(item, bytes):
                args[index] = item.decode(encoding)

    if compat_os_name != 'nt':
        return shlex.join(args)

    trans = _CMD_QUOTE_TRANS if shell else _WINDOWS_QUOTE_TRANS
    return ' '.join(
        s if re.fullmatch(r'[\w#$*\-+./:?@\\]+', s, re.ASCII) else s.translate(trans).join('""')
        for s in args)
    return shell_quote(args)