mirror of
https://github.com/alexta69/metube.git
synced 2026-07-17 16:47:30 +00:00
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.
This commit is contained in:
parent
3ea4732c5d
commit
fdfbfed5e2
2 changed files with 71 additions and 2 deletions
|
|
@ -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):
|
||||
|
|
|
|||
23
app/ytdl.py
23
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'
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue