CVE-2023-35934: Exposure of Sensitive Information to an Unauthorized Actor (CWE-200)
value: yt-dlp is a command-line program to download videos from video sites. During file downloads, yt-dlp or the external downloaders that yt-dlp employs may leak cookies on HTTP redirects to a different host, or leak them when the host for download fragments differs from their parent manifest's host. This vulnerable behavior is present in yt-dlp prior to 2023.07.06 and nightly 2023.07.06.185519. All native and external downloaders are affected, except for `curl` and `httpie` (version 3.1.0 or later).

At the file download stage, all cookies are passed by yt-dlp to the file downloader as a `Cookie` header, thereby losing their scope. This also occurs in yt-dlp's info JSON output, which may be used by external tools. As a result, the downloader or external tool may indiscriminately send cookies with requests to domains or paths for which the cookies are not scoped.

yt-dlp version 2023.07.06 and nightly 2023.07.06.185519 fix this issue by removing the `Cookie` header upon HTTP redirects; having native downloaders calculate the `Cookie` header from the cookiejar, utilizing external downloaders' built-in support for cookies instead of passing them as header arguments, disabling HTTP redirectiong if the external downloader does not have proper cookie support, processing cookies passed as HTTP headers to limit their scope, and having a separate field for cookies in the info dict storing more information about scoping

Some workarounds are available for those who are unable to upgrade. Avoid using cookies and user authentication methods. While extractors may set custom cookies, these usually do not contain sensitive information. Alternatively, avoid using `--load-info-json`. Or, if authentication is a must: verify the integrity of download links from unknown sources in browser (including redirects) before passing them to yt-dlp; use `curl` as external downloader, since it is not impacted; and/or avoid fragmented formats such as HLS/m3u8, DASH/mpd and ISM.
CWE description: The product exposes sensitive information to an actor that is not explicitly authorized to have access to that information.
CVSS v3 base score: 8.2
Published: 2023-07-06T20:15Z
Repository: https://github.com/yt-dlp/yt-dlp
Language: Python
Files changed: test/test_http.py, yt_dlp/utils/_utils.py
Commit date: 2023-07-06 23:14:39 +0530
Commit message: [core] Prevent `Cookie` leaks on HTTP redirect

Ref: https://github.com/yt-dlp/yt-dlp/security/advisories/GHSA-v8mc-9377-rwjj

Authored by: coletdjnz

Diff: diff --git a/test/test_http.py b/test/test_http.py
index 3941a6e77..e4e66dce1 100644
--- a/test/test_http.py
+++ b/test/test_http.py
@@ -130,10 +130,15 @@ def do_GET(self):
             self._redirect()
         elif self.path.startswith('/method'):
             self._method('GET')
         elif self.path.startswith('/headers'):
             self._headers()
+        elif self.path.startswith('/308-to-headers'):
+            self.send_response(308)
+            self.send_header('Location', '/headers')
+            self.send_header('Content-Length', '0')
+            self.end_headers()
         elif self.path == '/trailing_garbage':
             payload = b'<html><video src="/vid.mp4" /></html>'
             self.send_response(200)
             self.send_header('Content-Type', 'text/html; charset=utf-8')
             self.send_header('Content-Encoding', 'gzip')
@@ -268,10 +273,11 @@ def do_req(redirect_status, method):
             self.assertEqual(do_req(303, 'HEAD'), ('', 'HEAD'))
 
             self.assertEqual(do_req(303, 'PUT'), ('', 'GET'))
 
             # 301 and 302 turn POST only into a GET
+            # XXX: we should also test if the Content-Type and Content-Length headers are removed
             self.assertEqual(do_req(301, 'POST'), ('', 'GET'))
             self.assertEqual(do_req(301, 'HEAD'), ('', 'HEAD'))
             self.assertEqual(do_req(302, 'POST'), ('', 'GET'))
             self.assertEqual(do_req(302, 'HEAD'), ('', 'HEAD'))
 
@@ -311,10 +317,35 @@ def test_cookiejar(self):
                 0, 'test', 'ytdlp', None, False, '127.0.0.1', True,
                 False, '/headers', True, False, None, False, None, None, {}))
             data = ydl.urlopen(sanitized_Request(f'http://127.0.0.1:{self.http_port}/headers')).read()
             self.assertIn(b'Cookie: test=ytdlp', data)
 
+    def test_passed_cookie_header(self):
+        # We should accept a Cookie header being passed as in normal headers and handle it appropriately.
+        with FakeYDL() as ydl:
+            # Specified Cookie header should be used
+            res = ydl.urlopen(
+                sanitized_Request(f'http://127.0.0.1:{self.http_port}/headers',
+                                  headers={'Cookie': 'test=test'})).read().decode('utf-8')
+            self.assertIn('Cookie: test=test', res)
+
+            # Specified Cookie header should be removed on any redirect
+            res = ydl.urlopen(
+                sanitized_Request(f'http://127.0.0.1:{self.http_port}/308-to-headers', headers={'Cookie': 'test=test'})).read().decode('utf-8')
+            self.assertNotIn('Cookie: test=test', res)
+
+            # Specified Cookie header should override global cookiejar for that request
+            ydl.cookiejar.set_cookie(http.cookiejar.Cookie(
+                version=0, name='test', value='ytdlp', port=None, port_specified=False,
+                domain='127.0.0.1', domain_specified=True, domain_initial_dot=False, path='/',
+                path_specified=True, secure=False, expires=None, discard=False, comment=None,
+                comment_url=None, rest={}))
+
+            data = ydl.urlopen(sanitized_Request(f'http://127.0.0.1:{self.http_port}/headers', headers={'Cookie': 'test=test'})).read()
+            self.assertNotIn(b'Cookie: test=ytdlp', data)
+            self.assertIn(b'Cookie: test=test', data)
+
     def test_no_compression_compat_header(self):
         with FakeYDL() as ydl:
             data = ydl.urlopen(
                 sanitized_Request(
                     f'http://127.0.0.1:{self.http_port}/headers',
diff --git a/yt_dlp/utils/_utils.py b/yt_dlp/utils/_utils.py
index f68cdb968..82d9ba4d5 100644
--- a/yt_dlp/utils/_utils.py
+++ b/yt_dlp/utils/_utils.py
@@ -1554,11 +1554,16 @@ def redirect_request(self, req, fp, code, msg, headers, newurl):
         if code not in (301, 302, 303, 307, 308):
             raise urllib.error.HTTPError(req.full_url, code, msg, headers, fp)
 
         new_method = req.get_method()
         new_data = req.data
-        remove_headers = []
+
+        # Technically the Cookie header should be in unredirected_hdrs,
+        # however in practice some may set it in normal headers anyway.
+        # We will remove it here to prevent any leaks.
+        remove_headers = ['Cookie']
+
         # A 303 must either use GET or HEAD for subsequent request
         # https://datatracker.ietf.org/doc/html/rfc7231#section-6.4.4
         if code == 303 and req.get_method() != 'HEAD':
             new_method = 'GET'
         # 301 and 302 redirects are commonly turned into a GET from a POST
@@ -1571,11 +1576,11 @@ def redirect_request(self, req, fp, code, msg, headers, newurl):
         # only remove payload if method changed (e.g. POST to GET)
         if new_method != req.get_method():
             new_data = None
             remove_headers.extend(['Content-Length', 'Content-Type'])
 
-        new_headers = {k: v for k, v in req.headers.items() if k.lower() not in remove_headers}
+        new_headers = {k: v for k, v in req.headers.items() if k.title() not in remove_headers}
 
         return urllib.request.Request(
             newurl, headers=new_headers, origin_req_host=req.origin_req_host,
             unverifiable=True, method=new_method, data=new_data)
 

Vulnerable code:         remove_headers = []
        new_headers = {k: v for k, v in req.headers.items() if k.lower() not in remove_headers}
Fixed code:         elif self.path.startswith('/308-to-headers'):
            self.send_response(308)
            self.send_header('Location', '/headers')
            self.send_header('Content-Length', '0')
            self.end_headers()
            # XXX: we should also test if the Content-Type and Content-Length headers are removed
    def test_passed_cookie_header(self):
        # We should accept a Cookie header being passed as in normal headers and handle it appropriately.
        with FakeYDL() as ydl:
            # Specified Cookie header should be used
            res = ydl.urlopen(
                sanitized_Request(f'http://127.0.0.1:{self.http_port}/headers',
                                  headers={'Cookie': 'test=test'})).read().decode('utf-8')
            self.assertIn('Cookie: test=test', res)

            # Specified Cookie header should be removed on any redirect
            res = ydl.urlopen(
                sanitized_Request(f'http://127.0.0.1:{self.http_port}/308-to-headers', headers={'Cookie': 'test=test'})).read().decode('utf-8')
            self.assertNotIn('Cookie: test=test', res)

            # Specified Cookie header should override global cookiejar for that request
            ydl.cookiejar.set_cookie(http.cookiejar.Cookie(
                version=0, name='test', value='ytdlp', port=None, port_specified=False,
                domain='127.0.0.1', domain_specified=True, domain_initial_dot=False, path='/',
                path_specified=True, secure=False, expires=None, discard=False, comment=None,
                comment_url=None, rest={}))

            data = ydl.urlopen(sanitized_Request(f'http://127.0.0.1:{self.http_port}/headers', headers={'Cookie': 'test=test'})).read()
            self.assertNotIn(b'Cookie: test=ytdlp', data)
            self.assertIn(b'Cookie: test=test', data)


        # Technically the Cookie header should be in unredirected_hdrs,
        # however in practice some may set it in normal headers anyway.
        # We will remove it here to prevent any leaks.
        remove_headers = ['Cookie']

        new_headers = {k: v for k, v in req.headers.items() if k.title() not in remove_headers}