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_YoutubeDL.py, test/test_utils.py, yt_dlp/compat/__init__.py, yt_dlp/postprocessor/exec.py, yt_dlp/utils/_utils.py
Commit date: 2023-09-24 02:29:01 +0200
Commit message: [core] Prevent RCE when using `--exec` with `%q` (CVE-2023-40581)

The shell escape function is now using `""` instead of `\"`. `utils.Popen` has been patched to properly quote commands.

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-42h4-v29r-42qg for reference.

Authored by: Grub4K

Diff: diff --git a/devscripts/changelog_override.json b/devscripts/changelog_override.json
index 9dfbf510f7..fe0c82c66b 100644
--- a/devscripts/changelog_override.json
+++ b/devscripts/changelog_override.json
@@ -91,7 +91,12 @@
     },
     {
         "action": "add",
         "when": "c1d71d0d9f41db5e4306c86af232f5f6220a130b",
         "short": "[priority] **The minimum *recommended* Python version has been raised to 3.8**\nSince Python 3.7 has reached end-of-life, support for it will be dropped soon. [Read more](https://github.com/yt-dlp/yt-dlp/issues/7803)"
+    },
+    {
+        "action": "add",
+        "when": "61bdf15fc7400601c3da1aa7a43917310a5bf391",
+        "short": "[priority] Security: [[CVE-2023-40581](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-40581)] [Prevent RCE when using `--exec` with `%q` on Windows](https://github.com/yt-dlp/yt-dlp/security/advisories/GHSA-42h4-v29r-42qg)\n    - The shell escape function is now using `\"\"` instead of `\\\"`.\n    - `utils.Popen` has been patched to properly quote commands."
     }
 ]
diff --git a/test/test_YoutubeDL.py b/test/test_YoutubeDL.py
index 916ee48b97..0cf130db03 100644
--- a/test/test_YoutubeDL.py
+++ b/test/test_YoutubeDL.py
@@ -782,13 +782,13 @@ def expect_same_infodict(out):
         test('%(filesize)#D', '1Ki')
         test('%(height)5.2D', ' 1.08k')
         test('%(title4)#S', 'foo_bar_test')
         test('%(title4).10S', ('foo ＂bar＂ ', 'foo ＂bar＂' + ('#' if compat_os_name == 'nt' else ' ')))
         if compat_os_name == 'nt':
-            test('%(title4)q', ('"foo \\"bar\\" test"', "＂foo ⧹＂bar⧹＂ test＂"))
-            test('%(formats.:.id)#q', ('"id 1" "id 2" "id 3"', '＂id 1＂ ＂id 2＂ ＂id 3＂'))
-            test('%(formats.0.id)#q', ('"id 1"', '＂id 1＂'))
+            test('%(title4)q', ('"foo ""bar"" test"', None))
+            test('%(formats.:.id)#q', ('"id 1" "id 2" "id 3"', None))
+            test('%(formats.0.id)#q', ('"id 1"', None))
         else:
             test('%(title4)q', ('\'foo "bar" test\'', '\'foo ＂bar＂ test\''))
             test('%(formats.:.id)#q', "'id 1' 'id 2' 'id 3'")
             test('%(formats.0.id)#q', "'id 1'")
 
diff --git a/test/test_utils.py b/test/test_utils.py
index 47d1f71bfe..dc2d8ce12b 100644
--- a/test/test_utils.py
+++ b/test/test_utils.py
@@ -12,10 +12,11 @@
 
 import contextlib
 import io
 import itertools
 import json
+import subprocess
 import xml.etree.ElementTree
 
 from yt_dlp.compat import (
     compat_etree_fromstring,
     compat_HTMLParseError,
@@ -26,10 +27,11 @@
     DateRange,
     ExtractorError,
     InAdvancePagedList,
     LazyList,
     OnDemandPagedList,
+    Popen,
     age_restricted,
     args_to_str,
     base_url,
     caesar,
     clean_html,
@@ -2386,8 +2388,22 @@ def test_extract_basic_auth(self):
         assert extract_basic_auth('http://@foo.bar') == ('http://foo.bar', 'Basic Og==')
         assert extract_basic_auth('http://:pass@foo.bar') == ('http://foo.bar', 'Basic OnBhc3M=')
         assert extract_basic_auth('http://user:@foo.bar') == ('http://foo.bar', 'Basic dXNlcjo=')
         assert extract_basic_auth('http://user:pass@foo.bar') == ('http://foo.bar', 'Basic dXNlcjpwYXNz')
 
+    @unittest.skipUnless(compat_os_name == 'nt', 'Only relevant on Windows')
+    def test_Popen_windows_escaping(self):
+        def run_shell(args):
+            stdout, stderr, error = Popen.run(
+                args, text=True, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
+            assert not stderr
+            assert not error
+            return stdout
+
+        # Test escaping
+        assert run_shell(['echo', 'test"&']) == '"test""&"\n'
+        # Test if delayed expansion is disabled
+        assert run_shell(['echo', '^!']) == '"^!"\n'
+        assert run_shell('echo "^!"') == '"^!"\n'
 
 if __name__ == '__main__':
     unittest.main()
diff --git a/yt_dlp/compat/__init__.py b/yt_dlp/compat/__init__.py
index 832a9138d3..5ad5c70ecf 100644
--- a/yt_dlp/compat/__init__.py
+++ b/yt_dlp/compat/__init__.py
@@ -28,11 +28,11 @@ def compat_etree_fromstring(text):
 
 
 if compat_os_name == 'nt':
     def compat_shlex_quote(s):
         import re
-        return s if re.match(r'^[-_\w./]+$', s) else '"%s"' % s.replace('"', '\\"')
+        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_ord(c):
diff --git a/yt_dlp/postprocessor/exec.py b/yt_dlp/postprocessor/exec.py
index cfc83167ce..c2e73fbabd 100644
--- a/yt_dlp/postprocessor/exec.py
+++ b/yt_dlp/postprocessor/exec.py
@@ -1,10 +1,8 @@
-import subprocess
-
 from .common import PostProcessor
 from ..compat import compat_shlex_quote
-from ..utils import PostProcessingError, encodeArgument, variadic
+from ..utils import Popen, PostProcessingError, variadic
 
 
 class ExecPP(PostProcessor):
 
     def __init__(self, downloader, exec_cmd):
@@ -25,14 +23,14 @@ def parse_cmd(self, cmd, info):
         return cmd
 
     def run(self, info):
         for tmpl in self.exec_cmd:
             cmd = self.parse_cmd(tmpl, info)
-            self.to_screen('Executing command: %s' % cmd)
-            retCode = subprocess.call(encodeArgument(cmd), shell=True)
-            if retCode != 0:
-                raise PostProcessingError('Command returned error code %d' % retCode)
+            self.to_screen(f'Executing command: {cmd}')
+            _, _, return_code = Popen.run(cmd, shell=True)
+            if return_code != 0:
+                raise PostProcessingError(f'Command returned error code {return_code}')
         return [], info
 
 
 # Deprecated
 class ExecAfterDownloadPP(ExecPP):
diff --git a/yt_dlp/utils/_utils.py b/yt_dlp/utils/_utils.py
index 213ccc6363..ba62423806 100644
--- a/yt_dlp/utils/_utils.py
+++ b/yt_dlp/utils/_utils.py
@@ -823,21 +823,35 @@ def _fix(key):
                 env[key] = orig
 
         _fix('LD_LIBRARY_PATH')  # Linux
         _fix('DYLD_LIBRARY_PATH')  # macOS
 
-    def __init__(self, *args, env=None, text=False, **kwargs):
+    def __init__(self, args, *remaining, env=None, text=False, shell=False, **kwargs):
         if env is None:
             env = os.environ.copy()
         self._fix_pyinstaller_ld_path(env)
 
         self.__text_mode = kwargs.get('encoding') or kwargs.get('errors') or text or kwargs.get('universal_newlines')
         if text is True:
             kwargs['universal_newlines'] = True  # For 3.6 compatibility
             kwargs.setdefault('encoding', 'utf-8')
             kwargs.setdefault('errors', 'replace')
-        super().__init__(*args, env=env, **kwargs, startupinfo=self._startupinfo)
+
+        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)
+            shell = False
+            args = f'{self.__comspec()} /Q /S /D /V:OFF /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(
+            os.environ.get('SystemRoot', ''), 'System32', 'cmd.exe')
+        if os.path.isabs(comspec):
+            return comspec
+        raise FileNotFoundError('shell not found: neither %ComSpec% nor %SystemRoot% is set')
 
     def communicate_or_kill(self, *args, **kwargs):
         try:
             return self.communicate(*args, **kwargs)
         except BaseException:  # Including KeyboardInterrupt

Vulnerable code:             test('%(title4)q', ('"foo \\"bar\\" test"', "＂foo ⧹＂bar⧹＂ test＂"))
            test('%(formats.:.id)#q', ('"id 1" "id 2" "id 3"', '＂id 1＂ ＂id 2＂ ＂id 3＂'))
            test('%(formats.0.id)#q', ('"id 1"', '＂id 1＂'))
        return s if re.match(r'^[-_\w./]+$', s) else '"%s"' % s.replace('"', '\\"')
import subprocess

from ..utils import PostProcessingError, encodeArgument, variadic
            self.to_screen('Executing command: %s' % cmd)
            retCode = subprocess.call(encodeArgument(cmd), shell=True)
            if retCode != 0:
                raise PostProcessingError('Command returned error code %d' % retCode)
    def __init__(self, *args, env=None, text=False, **kwargs):
        super().__init__(*args, env=env, **kwargs, startupinfo=self._startupinfo)
Fixed code:     },
    {
        "action": "add",
        "when": "61bdf15fc7400601c3da1aa7a43917310a5bf391",
        "short": "[priority] Security: [[CVE-2023-40581](https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2023-40581)] [Prevent RCE when using `--exec` with `%q` on Windows](https://github.com/yt-dlp/yt-dlp/security/advisories/GHSA-42h4-v29r-42qg)\n    - The shell escape function is now using `\"\"` instead of `\\\"`.\n    - `utils.Popen` has been patched to properly quote commands."
            test('%(title4)q', ('"foo ""bar"" test"', None))
            test('%(formats.:.id)#q', ('"id 1" "id 2" "id 3"', None))
            test('%(formats.0.id)#q', ('"id 1"', None))
import subprocess
    Popen,
    @unittest.skipUnless(compat_os_name == 'nt', 'Only relevant on Windows')
    def test_Popen_windows_escaping(self):
        def run_shell(args):
            stdout, stderr, error = Popen.run(
                args, text=True, shell=True, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
            assert not stderr
            assert not error
            return stdout

        # Test escaping
        assert run_shell(['echo', 'test"&']) == '"test""&"\n'
        # Test if delayed expansion is disabled
        assert run_shell(['echo', '^!']) == '"^!"\n'
        assert run_shell('echo "^!"') == '"^!"\n'
        return s if re.match(r'^[-_\w./]+$', s) else s.replace('"', '""').join('""')
from ..utils import Popen, PostProcessingError, variadic
            self.to_screen(f'Executing command: {cmd}')
            _, _, return_code = Popen.run(cmd, shell=True)
            if return_code != 0:
                raise PostProcessingError(f'Command returned error code {return_code}')
    def __init__(self, args, *remaining, env=None, text=False, shell=False, **kwargs):

        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)
            shell = False
            args = f'{self.__comspec()} /Q /S /D /V:OFF /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(
            os.environ.get('SystemRoot', ''), 'System32', 'cmd.exe')
        if os.path.isabs(comspec):
            return comspec
        raise FileNotFoundError('shell not found: neither %ComSpec% nor %SystemRoot% is set')