CVE-2023-40581: Improper Neutralization of Special Elements used in an OS Command ('OS Command Injection') (CWE-78)
value: yt-dlp is a youtube-dl fork with additional features and fixes. yt-dlp allows the user to provide shell command lines to be executed at various stages in its download steps through the `--exec` flag. This flag allows output template expansion in its argument, so that metadata values may be used in the shell commands. The metadata fields can be combined with the `%q` conversion, which is intended to quote/escape these values so they can be safely passed to the shell. However, the escaping used for `cmd` (the shell used by Python's `subprocess` on Windows) does not properly escape special characters, which can allow for remote code execution if `--exec` is used directly with maliciously crafted remote data. This vulnerability only impacts `yt-dlp` on Windows, and the vulnerability is present regardless of whether `yt-dlp` is run from `cmd` or from `PowerShell`. 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 2023.09.24 fixes this issue by properly escaping each special character. `\n` will be replaced by `\r` as no way of escaping it has been found. It is recommended to upgrade yt-dlp to version 2023.09.24 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: 1. Avoid using any output template expansion in --exec other than {} (filepath). 2. If expansion in --exec is needed, verify the fields you are using do not contain ", | or &. 3. Instead of using --exec, write the info json and load the fields from it instead.

CWE description: The product constructs all or part of an OS command using externally-influenced input from an upstream component, but it does not neutralize or incorrectly neutralizes special elements that could modify the intended OS command when it is sent to a downstream component.
CVSS v3 base score: 7.8
Published: 2023-09-25T19: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 9dfbf510f..fe0c82c66 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 916ee48b9..0cf130db0 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 47d1f71bf..dc2d8ce12 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 832a9138d..5ad5c70ec 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 cfc83167c..c2e73fbab 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 213ccc636..ba6242380 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')