From fdfbfed5e205e8bca50022633c4f690c414960d3 Mon Sep 17 00:00:00 2001 From: Alex Shnitman Date: Mon, 13 Jul 2026 23:07:39 +0300 Subject: [PATCH] fix: prevent playlist/channel title path traversal (closes GHSA-vh67-38x4-w8pc) Sanitize path separators and .. segments in playlist/channel titles before they are baked into yt-dlp output templates, and refuse downloads whose resolved output directory escapes DOWNLOAD_DIR. --- app/tests/test_ytdl_utils.py | 50 ++++++++++++++++++++++++++++++++++++ app/ytdl.py | 23 +++++++++++++++-- 2 files changed, 71 insertions(+), 2 deletions(-) diff --git a/app/tests/test_ytdl_utils.py b/app/tests/test_ytdl_utils.py index 36c2451..f529574 100644 --- a/app/tests/test_ytdl_utils.py +++ b/app/tests/test_ytdl_utils.py @@ -43,6 +43,7 @@ from ytdl import ( DownloadInfo, _compact_persisted_entry, _convert_srt_to_txt_file, + _output_dir_escapes, _resolve_outtmpl_fields, _sanitize_entry_for_pickle, _sanitize_path_component, @@ -61,6 +62,24 @@ class SanitizePathComponentTests(unittest.TestCase): self.assertIs(_sanitize_path_component(None), None) self.assertEqual(_sanitize_path_component(42), 42) + def test_strips_path_separators_and_traversal(self): + result = _sanitize_path_component('../../../../etc/x') + self.assertNotIn('..', result) + self.assertNotIn('/', result) + self.assertNotIn('\\', result) + + def test_strips_leading_absolute_path_separator(self): + result = _sanitize_path_component('/tmp/x') + self.assertFalse(result.startswith('/')) + self.assertFalse(result.startswith('\\')) + self.assertEqual(result, '_tmp_x') + + def test_collapses_slashes_in_legitimate_titles(self): + self.assertEqual(_sanitize_path_component('AC/DC'), 'AC_DC') + + def test_empty_after_strip_becomes_underscore(self): + self.assertEqual(_sanitize_path_component(' '), '_') + @unittest.skipUnless(_has_real_ytdlp, "requires real yt-dlp") class ResolveOuttmplFieldsTests(unittest.TestCase): @@ -125,6 +144,37 @@ class ResolveOuttmplFieldsTests(unittest.TestCase): ) self.assertEqual(result, "5 - %(title)s.%(ext)s") + def test_malicious_playlist_title_cannot_escape_via_template(self): + malicious_title = '/tmp/METUBE_ARBITRARY_WRITE_POC' + entry = { + 'playlist_title': malicious_title, + 'playlist_index': '1', + 'title': 'video', + 'ext': 'mp4', + } + sanitized = {k: _sanitize_path_component(v) for k, v in entry.items()} + template = '%(playlist_title)s/%(title)s.%(ext)s' + result = _resolve_outtmpl_fields(template, sanitized, ('playlist',)) + marker = result.find('%(') + literal_prefix = result[:marker] if marker != -1 else result + self.assertNotIn('..', literal_prefix) + self.assertFalse(literal_prefix.startswith('/')) + self.assertFalse(literal_prefix.startswith('\\')) + + +class OutputDirEscapesTests(unittest.TestCase): + def setUp(self): + self.base_dir = tempfile.mkdtemp() + + def test_relative_traversal_escapes(self): + self.assertTrue(_output_dir_escapes(self.base_dir, '../../tmp/x/%(title)s.%(ext)s')) + + def test_absolute_path_escapes(self): + self.assertTrue(_output_dir_escapes(self.base_dir, '/tmp/x/%(title)s.%(ext)s')) + + def test_normal_playlist_dir_stays_inside(self): + self.assertFalse(_output_dir_escapes(self.base_dir, 'Playlist/%(title)s.%(ext)s')) + class SanitizeEntryForPickleTests(unittest.TestCase): def test_nested(self): diff --git a/app/ytdl.py b/app/ytdl.py index 43a7772..8a2dc42 100644 --- a/app/ytdl.py +++ b/app/ytdl.py @@ -72,6 +72,7 @@ def _is_within_directory(real_base: str, real_target: str) -> bool: # sanitised when substituting playlist/channel titles into output templates so # that downloads do not fail on NTFS-mounted volumes or Windows Docker hosts. _WINDOWS_INVALID_PATH_CHARS = re.compile(r'[\\:*?"<>|]') +_PATH_SEP_OR_TRAVERSAL = re.compile(r'[\\/]|\.\.') def _sanitize_path_component(value: Any) -> Any: @@ -81,11 +82,27 @@ def _sanitize_path_component(value: Any) -> Any: that numeric format specs (e.g. ``%(playlist_index)02d``) still work. Only string values are sanitised because Windows-invalid characters are only a concern for human-readable strings (titles, channel names, etc.) - that may end up as directory names. + that may end up as directory names. Path separators and ``..`` segments + are also collapsed so attacker-controlled playlist/channel titles cannot + escape the download directory via the output template. """ if not isinstance(value, str): return value - return _WINDOWS_INVALID_PATH_CHARS.sub('_', value) + value = _WINDOWS_INVALID_PATH_CHARS.sub('_', value) + value = _PATH_SEP_OR_TRAVERSAL.sub('_', value) + return value.lstrip('.').strip() or '_' + + +def _output_dir_escapes(base_dir: str, output_template: str) -> bool: + """True when the literal directory prefix of *output_template* resolves outside *base_dir*.""" + marker = output_template.find('%(') + literal = output_template if marker == -1 else output_template[:marker] + dir_prefix = os.path.dirname(literal) + if not dir_prefix: + return False + real_base = os.path.realpath(base_dir) + real_target = os.path.realpath(os.path.join(base_dir, dir_prefix)) + return not _is_within_directory(real_base, real_target) # Regex matching yt-dlp output-template field references, e.g. ``%(title)s`` @@ -1205,6 +1222,8 @@ class DownloadQueue: if playlist_item_limit > 0: log.info(f'playlist limit is set. Processing only first {playlist_item_limit} entries') ytdl_options['playlistend'] = playlist_item_limit + if _output_dir_escapes(dldirectory, output): + return {'status': 'error', 'msg': 'Refusing download: resolved output path escapes the download directory'} download = Download(dldirectory, self.config.TEMP_DIR, output, output_chapter, dl.quality, dl.format, ytdl_options, dl) is_upcoming = ( getattr(dl, 'live_status', None) == 'is_upcoming'