From e37afbe0b8a1222cb214ad0bec9a53bb7953531d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Marqui=CC=81nez=20Ferra=CC=81ndiz?= Date: Thu, 14 Jan 2016 00:16:23 +0100 Subject: [PATCH 1/4] [YoutubeDL] urlopen: disable the 'file:' protocol (#8227) If someone is running youtube-dl on a server to deliver files, the user could input 'file:///some/important/file' and youtube-dl would save that file as a video giving access to sensitive information to the user. 'file:' urls can be filtered, but the user can use an URL to a crafted m3u8 manifest like: #EXTM3U #EXT-X-MEDIA-SEQUENCE:0 #EXTINF:10.0 file:///etc/passwd #EXT-X-ENDLIST With this patch 'file:' URLs raise URLError like for unknown protocols. --- test/test_YoutubeDL.py | 7 ++++++- youtube_dl/YoutubeDL.py | 10 ++++++++-- 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/test/test_YoutubeDL.py b/test/test_YoutubeDL.py index 0388c0bf32..0caa43843a 100644 --- a/test/test_YoutubeDL.py +++ b/test/test_YoutubeDL.py @@ -12,7 +12,7 @@ import copy from test.helper import FakeYDL, assertRegexpMatches from youtube_dl import YoutubeDL -from youtube_dl.compat import compat_str +from youtube_dl.compat import compat_str, compat_urllib_error from youtube_dl.extractor import YoutubeIE from youtube_dl.postprocessor.common import PostProcessor from youtube_dl.utils import ExtractorError, match_filter_func @@ -631,6 +631,11 @@ class TestYoutubeDL(unittest.TestCase): result = get_ids({'playlist_items': '10'}) self.assertEqual(result, []) + def test_urlopen_no_file_protocol(self): + # see https://github.com/rg3/youtube-dl/issues/8227 + ydl = YDL() + self.assertRaises(compat_urllib_error.URLError, ydl.urlopen, 'file:///etc/passwd') + if __name__ == '__main__': unittest.main() diff --git a/youtube_dl/YoutubeDL.py b/youtube_dl/YoutubeDL.py index d50b7cfed3..e8ce586042 100755 --- a/youtube_dl/YoutubeDL.py +++ b/youtube_dl/YoutubeDL.py @@ -1986,8 +1986,14 @@ class YoutubeDL(object): https_handler = make_HTTPS_handler(self.params, debuglevel=debuglevel) ydlh = YoutubeDLHandler(self.params, debuglevel=debuglevel) data_handler = compat_urllib_request_DataHandler() - opener = compat_urllib_request.build_opener( - proxy_handler, https_handler, cookie_processor, ydlh, data_handler) + unknown_handler = compat_urllib_request.UnknownHandler() + handlers = (proxy_handler, https_handler, cookie_processor, ydlh, data_handler, unknown_handler) + # we don't use build_opener because it automatically adds FileHandler, + # which can be used for malicious purposes (see + # https://github.com/rg3/youtube-dl/issues/8227) + opener = compat_urllib_request.OpenerDirector() + for handler in handlers: + opener.add_handler(handler) # Delete the default user-agent header, which would otherwise apply in # cases where our custom HTTP handler doesn't come into play From 6240b0a278781a3b584a9dd6d57191b2472c0fd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Marqui=CC=81nez=20Ferra=CC=81ndiz?= Date: Thu, 14 Jan 2016 08:14:01 +0100 Subject: [PATCH 2/4] [YoutubeDL] urlopen: use build_opener again Otherwise we would need to manually add handlers like HTTPRedirectHandler, instead we add a customized FileHandler instance that raises an error. --- youtube_dl/YoutubeDL.py | 19 ++++++++++++------- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/youtube_dl/YoutubeDL.py b/youtube_dl/YoutubeDL.py index e8ce586042..ccad5f2eaa 100755 --- a/youtube_dl/YoutubeDL.py +++ b/youtube_dl/YoutubeDL.py @@ -1986,14 +1986,19 @@ class YoutubeDL(object): https_handler = make_HTTPS_handler(self.params, debuglevel=debuglevel) ydlh = YoutubeDLHandler(self.params, debuglevel=debuglevel) data_handler = compat_urllib_request_DataHandler() - unknown_handler = compat_urllib_request.UnknownHandler() - handlers = (proxy_handler, https_handler, cookie_processor, ydlh, data_handler, unknown_handler) - # we don't use build_opener because it automatically adds FileHandler, - # which can be used for malicious purposes (see + + # When passing our own FileHandler instance, build_opener won't add the + # default FileHandler and allows us to disable the file protocol, which + # can be used for malicious purposes (see # https://github.com/rg3/youtube-dl/issues/8227) - opener = compat_urllib_request.OpenerDirector() - for handler in handlers: - opener.add_handler(handler) + file_handler = compat_urllib_request.FileHandler() + + def file_open(*args, **kwargs): + raise compat_urllib_error.URLError('file protocol is disabled') + file_handler.file_open = file_open + + opener = compat_urllib_request.build_opener( + proxy_handler, https_handler, cookie_processor, ydlh, data_handler, file_handler) # Delete the default user-agent header, which would otherwise apply in # cases where our custom HTTP handler doesn't come into play From 4240d504963bb6d1c7bd7c288a7874f9d8dc042b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Marqui=CC=81nez=20Ferra=CC=81ndiz?= Date: Thu, 14 Jan 2016 14:07:54 +0100 Subject: [PATCH 3/4] [YoutubeDL] improve error message for file:/// URLs --- youtube_dl/YoutubeDL.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/youtube_dl/YoutubeDL.py b/youtube_dl/YoutubeDL.py index ccad5f2eaa..4915fbd45f 100755 --- a/youtube_dl/YoutubeDL.py +++ b/youtube_dl/YoutubeDL.py @@ -1994,7 +1994,7 @@ class YoutubeDL(object): file_handler = compat_urllib_request.FileHandler() def file_open(*args, **kwargs): - raise compat_urllib_error.URLError('file protocol is disabled') + raise compat_urllib_error.URLError('file:/// protocol is explicitly disabled in youtube-dl for security reasons') file_handler.file_open = file_open opener = compat_urllib_request.build_opener( From 30e2f2d76f6dd52803effce14fa14f3a8051c84a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaime=20Marqui=CC=81nez=20Ferra=CC=81ndiz?= Date: Thu, 14 Jan 2016 16:28:46 +0100 Subject: [PATCH 4/4] [YoutubeDL] use a more correct terminology in the error message for file:// URLs --- youtube_dl/YoutubeDL.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/youtube_dl/YoutubeDL.py b/youtube_dl/YoutubeDL.py index 4915fbd45f..6b73b8e06e 100755 --- a/youtube_dl/YoutubeDL.py +++ b/youtube_dl/YoutubeDL.py @@ -1994,7 +1994,7 @@ class YoutubeDL(object): file_handler = compat_urllib_request.FileHandler() def file_open(*args, **kwargs): - raise compat_urllib_error.URLError('file:/// protocol is explicitly disabled in youtube-dl for security reasons') + raise compat_urllib_error.URLError('file:// scheme is explicitly disabled in youtube-dl for security reasons') file_handler.file_open = file_open opener = compat_urllib_request.build_opener(