[core] Disallow unsafe extensions (CVE-2024-38519)

Ref: https://github.com/yt-dlp/yt-dlp/security/advisories/GHSA-79w7-vh3h-8g4j

Authored by: Grub4K
This commit is contained in:
Simon Sawicki 2024-07-02 00:52:50 +02:00
parent 6aaf96a3d6
commit 5ce582448e
No known key found for this signature in database
7 changed files with 179 additions and 12 deletions

View File

@ -2229,6 +2229,14 @@ For ease of use, a few more compat options are available:
* `--compat-options 2022`: Same as `--compat-options 2023,playlist-match-filter,no-external-downloader-progress,prefer-legacy-http-handler,manifest-filesize-approx` * `--compat-options 2022`: Same as `--compat-options 2023,playlist-match-filter,no-external-downloader-progress,prefer-legacy-http-handler,manifest-filesize-approx`
* `--compat-options 2023`: Currently does nothing. Use this to enable all future compat options * `--compat-options 2023`: Currently does nothing. Use this to enable all future compat options
The following compat options restore vulnerable behavior from before security patches:
* `--compat-options allow-unsafe-ext`: Allow files with any extension (including unsafe ones) to be downloaded ([GHSA-79w7-vh3h-8g4j](<https://github.com/yt-dlp/yt-dlp/security/advisories/GHSA-79w7-vh3h-8g4j>))
> :warning: Only use if a valid file download is rejected because its extension is detected as uncommon
>
> **This option can enable remote code execution! Consider [opening an issue](<https://github.com/yt-dlp/yt-dlp/issues/new/choose>) instead!**
### Deprecated options ### Deprecated options
These are all the deprecated options and the current alternative to achieve the same effect These are all the deprecated options and the current alternative to achieve the same effect

View File

@ -175,5 +175,10 @@
"when": "e6a22834df1776ec4e486526f6df2bf53cb7e06f", "when": "e6a22834df1776ec4e486526f6df2bf53cb7e06f",
"short": "[ie/orf:on] Add `prefer_segments_playlist` extractor-arg (#10314)", "short": "[ie/orf:on] Add `prefer_segments_playlist` extractor-arg (#10314)",
"authors": ["seproDev"] "authors": ["seproDev"]
},
{
"action": "add",
"when": "6aaf96a3d6e7d0d426e97e11a2fcf52fda00e733",
"short": "[priority] Security: [[CVE-2024-10123](https://nvd.nist.gov/vuln/detail/CVE-2024-10123)] [Properly sanitize file-extension to prevent file system modification and RCE](https://github.com/yt-dlp/yt-dlp/security/advisories/GHSA-79w7-vh3h-8g4j)\n - Unsafe extensions are now blocked from being downloaded"
} }
] ]

View File

@ -130,6 +130,7 @@ from yt_dlp.utils import (
xpath_text, xpath_text,
xpath_with_ns, xpath_with_ns,
) )
from yt_dlp.utils._utils import _UnsafeExtensionError
from yt_dlp.utils.networking import ( from yt_dlp.utils.networking import (
HTTPHeaderDict, HTTPHeaderDict,
escape_rfc3986, escape_rfc3986,
@ -281,6 +282,13 @@ class TestUtil(unittest.TestCase):
finally: finally:
os.environ['HOME'] = old_home or '' os.environ['HOME'] = old_home or ''
_uncommon_extensions = [
('exe', 'abc.exe.ext'),
('de', 'abc.de.ext'),
('../.mp4', None),
('..\\.mp4', None),
]
def test_prepend_extension(self): def test_prepend_extension(self):
self.assertEqual(prepend_extension('abc.ext', 'temp'), 'abc.temp.ext') self.assertEqual(prepend_extension('abc.ext', 'temp'), 'abc.temp.ext')
self.assertEqual(prepend_extension('abc.ext', 'temp', 'ext'), 'abc.temp.ext') self.assertEqual(prepend_extension('abc.ext', 'temp', 'ext'), 'abc.temp.ext')
@ -289,6 +297,19 @@ class TestUtil(unittest.TestCase):
self.assertEqual(prepend_extension('.abc', 'temp'), '.abc.temp') self.assertEqual(prepend_extension('.abc', 'temp'), '.abc.temp')
self.assertEqual(prepend_extension('.abc.ext', 'temp'), '.abc.temp.ext') self.assertEqual(prepend_extension('.abc.ext', 'temp'), '.abc.temp.ext')
# Test uncommon extensions
self.assertEqual(prepend_extension('abc.ext', 'bin'), 'abc.bin.ext')
for ext, result in self._uncommon_extensions:
with self.assertRaises(_UnsafeExtensionError):
prepend_extension('abc', ext)
if result:
self.assertEqual(prepend_extension('abc.ext', ext, 'ext'), result)
else:
with self.assertRaises(_UnsafeExtensionError):
prepend_extension('abc.ext', ext, 'ext')
with self.assertRaises(_UnsafeExtensionError):
prepend_extension('abc.unexpected_ext', ext, 'ext')
def test_replace_extension(self): def test_replace_extension(self):
self.assertEqual(replace_extension('abc.ext', 'temp'), 'abc.temp') self.assertEqual(replace_extension('abc.ext', 'temp'), 'abc.temp')
self.assertEqual(replace_extension('abc.ext', 'temp', 'ext'), 'abc.temp') self.assertEqual(replace_extension('abc.ext', 'temp', 'ext'), 'abc.temp')
@ -297,6 +318,16 @@ class TestUtil(unittest.TestCase):
self.assertEqual(replace_extension('.abc', 'temp'), '.abc.temp') self.assertEqual(replace_extension('.abc', 'temp'), '.abc.temp')
self.assertEqual(replace_extension('.abc.ext', 'temp'), '.abc.temp') self.assertEqual(replace_extension('.abc.ext', 'temp'), '.abc.temp')
# Test uncommon extensions
self.assertEqual(replace_extension('abc.ext', 'bin'), 'abc.unknown_video')
for ext, _ in self._uncommon_extensions:
with self.assertRaises(_UnsafeExtensionError):
replace_extension('abc', ext)
with self.assertRaises(_UnsafeExtensionError):
replace_extension('abc.ext', ext, 'ext')
with self.assertRaises(_UnsafeExtensionError):
replace_extension('abc.unexpected_ext', ext, 'ext')
def test_subtitles_filename(self): def test_subtitles_filename(self):
self.assertEqual(subtitles_filename('abc.ext', 'en', 'vtt'), 'abc.en.vtt') self.assertEqual(subtitles_filename('abc.ext', 'en', 'vtt'), 'abc.en.vtt')
self.assertEqual(subtitles_filename('abc.ext', 'en', 'vtt', 'ext'), 'abc.en.vtt') self.assertEqual(subtitles_filename('abc.ext', 'en', 'vtt', 'ext'), 'abc.en.vtt')

View File

@ -159,7 +159,7 @@ from .utils import (
write_json_file, write_json_file,
write_string, write_string,
) )
from .utils._utils import _YDLLogger from .utils._utils import _UnsafeExtensionError, _YDLLogger
from .utils.networking import ( from .utils.networking import (
HTTPHeaderDict, HTTPHeaderDict,
clean_headers, clean_headers,
@ -172,6 +172,20 @@ if compat_os_name == 'nt':
import ctypes import ctypes
def _catch_unsafe_extension_error(func):
@functools.wraps(func)
def wrapper(self, *args, **kwargs):
try:
return func(self, *args, **kwargs)
except _UnsafeExtensionError as error:
self.report_error(
f'The extracted extension ({error.extension!r}) is unusual '
'and will be skipped for safety reasons. '
f'If you believe this is an error{bug_reports_message(",")}')
return wrapper
class YoutubeDL: class YoutubeDL:
"""YoutubeDL class. """YoutubeDL class.
@ -454,8 +468,9 @@ class YoutubeDL:
Set the value to 'native' to use the native downloader Set the value to 'native' to use the native downloader
compat_opts: Compatibility options. See "Differences in default behavior". compat_opts: Compatibility options. See "Differences in default behavior".
The following options do not work when used through the API: The following options do not work when used through the API:
filename, abort-on-error, multistreams, no-live-chat, format-sort filename, abort-on-error, multistreams, no-live-chat,
no-clean-infojson, no-playlist-metafiles, no-keep-subs, no-attach-info-json. format-sort, no-clean-infojson, no-playlist-metafiles,
no-keep-subs, no-attach-info-json, allow-unsafe-ext.
Refer __init__.py for their implementation Refer __init__.py for their implementation
progress_template: Dictionary of templates for progress outputs. progress_template: Dictionary of templates for progress outputs.
Allowed keys are 'download', 'postprocess', Allowed keys are 'download', 'postprocess',
@ -1400,6 +1415,7 @@ class YoutubeDL:
outtmpl, info_dict = self.prepare_outtmpl(outtmpl, info_dict, *args, **kwargs) outtmpl, info_dict = self.prepare_outtmpl(outtmpl, info_dict, *args, **kwargs)
return self.escape_outtmpl(outtmpl) % info_dict return self.escape_outtmpl(outtmpl) % info_dict
@_catch_unsafe_extension_error
def _prepare_filename(self, info_dict, *, outtmpl=None, tmpl_type=None): def _prepare_filename(self, info_dict, *, outtmpl=None, tmpl_type=None):
assert None in (outtmpl, tmpl_type), 'outtmpl and tmpl_type are mutually exclusive' assert None in (outtmpl, tmpl_type), 'outtmpl and tmpl_type are mutually exclusive'
if outtmpl is None: if outtmpl is None:
@ -3192,6 +3208,7 @@ class YoutubeDL:
os.remove(file) os.remove(file)
return None return None
@_catch_unsafe_extension_error
def process_info(self, info_dict): def process_info(self, info_dict):
"""Process a single resolved IE result. (Modifies it in-place)""" """Process a single resolved IE result. (Modifies it in-place)"""

View File

@ -64,6 +64,7 @@ from .utils import (
write_string, write_string,
) )
from .utils.networking import std_headers from .utils.networking import std_headers
from .utils._utils import _UnsafeExtensionError
from .YoutubeDL import YoutubeDL from .YoutubeDL import YoutubeDL
_IN_CLI = False _IN_CLI = False
@ -593,6 +594,13 @@ def validate_options(opts):
if opts.ap_username is not None and opts.ap_password is None: if opts.ap_username is not None and opts.ap_password is None:
opts.ap_password = getpass.getpass('Type TV provider account password and press [Return]: ') opts.ap_password = getpass.getpass('Type TV provider account password and press [Return]: ')
# compat option changes global state destructively; only allow from cli
if 'allow-unsafe-ext' in opts.compat_opts:
warnings.append(
'Using allow-unsafe-ext opens you up to potential attacks. '
'Use with great care!')
_UnsafeExtensionError.sanitize_extension = lambda x: x
return warnings, deprecation_warnings return warnings, deprecation_warnings

View File

@ -474,7 +474,7 @@ def create_parser():
'no-attach-info-json', 'embed-thumbnail-atomicparsley', 'no-external-downloader-progress', 'no-attach-info-json', 'embed-thumbnail-atomicparsley', 'no-external-downloader-progress',
'embed-metadata', 'seperate-video-versions', 'no-clean-infojson', 'no-keep-subs', 'no-certifi', 'embed-metadata', 'seperate-video-versions', 'no-clean-infojson', 'no-keep-subs', 'no-certifi',
'no-youtube-channel-redirect', 'no-youtube-unavailable-videos', 'no-youtube-prefer-utc-upload-date', 'no-youtube-channel-redirect', 'no-youtube-unavailable-videos', 'no-youtube-prefer-utc-upload-date',
'prefer-legacy-http-handler', 'manifest-filesize-approx', 'prefer-legacy-http-handler', 'manifest-filesize-approx', 'allow-unsafe-ext',
}, 'aliases': { }, 'aliases': {
'youtube-dl': ['all', '-multistreams', '-playlist-match-filter', '-manifest-filesize-approx'], 'youtube-dl': ['all', '-multistreams', '-playlist-match-filter', '-manifest-filesize-approx'],
'youtube-dlc': ['all', '-no-youtube-channel-redirect', '-no-live-chat', '-playlist-match-filter', '-manifest-filesize-approx'], 'youtube-dlc': ['all', '-no-youtube-channel-redirect', '-no-live-chat', '-playlist-match-filter', '-manifest-filesize-approx'],

View File

@ -2085,17 +2085,20 @@ def parse_duration(s):
(days, 86400), (hours, 3600), (mins, 60), (secs, 1), (ms, 1))) (days, 86400), (hours, 3600), (mins, 60), (secs, 1), (ms, 1)))
def prepend_extension(filename, ext, expected_real_ext=None): def _change_extension(prepend, filename, ext, expected_real_ext=None):
name, real_ext = os.path.splitext(filename) name, real_ext = os.path.splitext(filename)
return (
f'{name}.{ext}{real_ext}' if not expected_real_ext or real_ext[1:] == expected_real_ext:
if not expected_real_ext or real_ext[1:] == expected_real_ext filename = name
else f'{filename}.{ext}') if prepend and real_ext:
_UnsafeExtensionError.sanitize_extension(ext, prepend=True)
return f'{filename}.{ext}{real_ext}'
return f'{filename}.{_UnsafeExtensionError.sanitize_extension(ext)}'
def replace_extension(filename, ext, expected_real_ext=None): prepend_extension = functools.partial(_change_extension, True)
name, real_ext = os.path.splitext(filename) replace_extension = functools.partial(_change_extension, False)
return f'{name if not expected_real_ext or real_ext[1:] == expected_real_ext else filename}.{ext}'
def check_executable(exe, args=[]): def check_executable(exe, args=[]):
@ -5035,6 +5038,101 @@ MEDIA_EXTENSIONS.audio += MEDIA_EXTENSIONS.common_audio
KNOWN_EXTENSIONS = (*MEDIA_EXTENSIONS.video, *MEDIA_EXTENSIONS.audio, *MEDIA_EXTENSIONS.manifests) KNOWN_EXTENSIONS = (*MEDIA_EXTENSIONS.video, *MEDIA_EXTENSIONS.audio, *MEDIA_EXTENSIONS.manifests)
class _UnsafeExtensionError(Exception):
"""
Mitigation exception for uncommon/malicious file extensions
This should be caught in YoutubeDL.py alongside a warning
Ref: https://github.com/yt-dlp/yt-dlp/security/advisories/GHSA-79w7-vh3h-8g4j
"""
ALLOWED_EXTENSIONS = frozenset([
# internal
'description',
'json',
'meta',
'orig',
'part',
'temp',
'uncut',
'unknown_video',
'ytdl',
# video
*MEDIA_EXTENSIONS.video,
'avif',
'ismv',
'm2ts',
'm4s',
'mng',
'mpeg',
'qt',
'swf',
'ts',
'vp9',
'wvm',
# audio
*MEDIA_EXTENSIONS.audio,
'isma',
'mid',
'mpga',
'ra',
# image
*MEDIA_EXTENSIONS.thumbnails,
'bmp',
'gif',
'heic',
'ico',
'jng',
'jpeg',
'jxl',
'svg',
'tif',
'wbmp',
# subtitle
*MEDIA_EXTENSIONS.subtitles,
'dfxp',
'fs',
'ismt',
'sami',
'scc',
'ssa',
'tt',
'ttml',
# others
*MEDIA_EXTENSIONS.manifests,
*MEDIA_EXTENSIONS.storyboards,
'desktop',
'ism',
'm3u',
'sbv',
'url',
'webloc',
'xml',
])
def __init__(self, extension, /):
super().__init__(f'unsafe file extension: {extension!r}')
self.extension = extension
@classmethod
def sanitize_extension(cls, extension, /, *, prepend=False):
if '/' in extension or '\\' in extension:
raise cls(extension)
if not prepend:
_, _, last = extension.rpartition('.')
if last == 'bin':
extension = last = 'unknown_video'
if last.lower() not in cls.ALLOWED_EXTENSIONS:
raise cls(extension)
return extension
class RetryManager: class RetryManager:
"""Usage: """Usage:
for retry in RetryManager(...): for retry in RetryManager(...):