From f5578a7931076922b80582a3373e6a47bc9d5fe1 Mon Sep 17 00:00:00 2001 From: MoojMidge <56883549+MoojMidge@users.noreply.github.com> Date: Fri, 6 Dec 2024 12:46:11 +1100 Subject: [PATCH] Fix potential leak of sensitive data via HTTPServer logging #1016 --- .../kodion/network/http_server.py | 83 +++++++++++++------ .../youtube_plugin/kodion/utils/methods.py | 3 +- 2 files changed, 60 insertions(+), 26 deletions(-) diff --git a/resources/lib/youtube_plugin/kodion/network/http_server.py b/resources/lib/youtube_plugin/kodion/network/http_server.py index 8f4a1b4b2..c0fa89aa8 100644 --- a/resources/lib/youtube_plugin/kodion/network/http_server.py +++ b/resources/lib/youtube_plugin/kodion/network/http_server.py @@ -21,6 +21,7 @@ BaseHTTPRequestHandler, TCPServer, parse_qsl, + urlencode, urlsplit, urlunsplit, xbmc, @@ -92,25 +93,58 @@ def connection_allowed(self, method): else: in_local_range = 'Undetermined' - if self.path != PATHS.PING: + path_parts = urlsplit(self.path) + if path_parts.query: + params = dict(parse_qsl(path_parts.query)) + log_params = params.copy() + for param, value in params.items(): + if param in {'key', 'api_key', 'api_secret', 'client_secret'}: + log_params[param] = '...'.join((value[:3], value[-3:])) + elif param in {'api_id', 'client_id'}: + log_params[param] = '...'.join((value[:3], value[-5:])) + elif param in {'access_token', 'refresh_token', 'token'}: + log_params[param] = '' + elif param == 'url': + log_params[param] = redact_ip(value) + elif param == 'location': + log_params[param] = '|xx.xxxx,xx.xxxx|' + log_path = urlunsplit(( + '', '', path_parts.path, urlencode(log_params), '', + )) + else: + params = log_params = None + log_path = path_parts.path + path = { + 'full': self.path, + 'path': path_parts.path, + 'query': path_parts.query, + 'params': params, + 'log_params': log_params, + 'log_path': log_path, + } + + if not path['path'].startswith(PATHS.PING): msg = ('HTTPServer - {method}' '\n\tPath: |{path}|' + '\n\tParams: |{params}|' '\n\tAddress: |{client_ip}|' '\n\tWhitelisted: {is_whitelisted}' '\n\tLocal range: {in_local_range}' '\n\tStatus: {status}' .format(method=method, - path=redact_ip(self.path), + path=path['path'], + params=path['log_params'], client_ip=client_ip, is_whitelisted=is_whitelisted, in_local_range=in_local_range, status='Allowed' if conn_allowed else 'Blocked')) self._context.log_debug(msg) - return conn_allowed + return conn_allowed, path # noinspection PyPep8Naming def do_GET(self): - if not self.connection_allowed('GET'): + allowed, path = self.connection_allowed('GET') + if not allowed: self.send_error(403) return @@ -120,10 +154,7 @@ def do_GET(self): settings = context.get_settings() api_config_enabled = settings.api_config_page() - # Strip trailing slash if present - stripped_path = self.path.rstrip('/') - - if stripped_path == PATHS.IP: + if path['path'] == PATHS.IP: client_json = json.dumps({'ip': self.client_address[0]}) self.send_response(200) self.send_header('Content-Type', 'application/json; charset=utf-8') @@ -131,9 +162,9 @@ def do_GET(self): self.end_headers() self.wfile.write(client_json.encode('utf-8')) - elif stripped_path.startswith(PATHS.MPD): + elif path['path'].startswith(PATHS.MPD): try: - file = dict(parse_qsl(urlsplit(self.path).query)).get('file') + file = path['params'].get('file') if file: filepath = os.path.join(self.BASE_PATH, file) else: @@ -154,10 +185,10 @@ def do_GET(self): self.wfile.write(file_chunk) except IOError: response = ('File Not Found: |{path}| -> |{filepath}|' - .format(path=self.path, filepath=filepath)) + .format(path=path['log_path'], filepath=filepath)) self.send_error(404, response) - elif api_config_enabled and stripped_path == PATHS.API: + elif api_config_enabled and path['path'] == PATHS.API: html = self.api_config_page() html = html.encode('utf-8') @@ -169,11 +200,11 @@ def do_GET(self): for chunk in self.get_chunks(html): self.wfile.write(chunk) - elif api_config_enabled and stripped_path.startswith(PATHS.API_SUBMIT): + elif api_config_enabled and path['path'].startswith(PATHS.API_SUBMIT): xbmc.executebuiltin('Dialog.Close(addonsettings,true)') - query = urlsplit(self.path).query - params = dict(parse_qsl(query)) + query = path['query'] + params = path['params'] updated = [] api_key = params.get('api_key') @@ -227,11 +258,11 @@ def do_GET(self): for chunk in self.get_chunks(html): self.wfile.write(chunk) - elif stripped_path == PATHS.PING: + elif path['path'] == PATHS.PING: self.send_error(204) - elif stripped_path.startswith(PATHS.REDIRECT): - url = dict(parse_qsl(urlsplit(self.path).query)).get('url') + elif path['path'].startswith(PATHS.REDIRECT): + url = path['params'].get('url') if url: wait(1) self.send_response(301) @@ -246,13 +277,14 @@ def do_GET(self): # noinspection PyPep8Naming def do_HEAD(self): - if not self.connection_allowed('HEAD'): + allowed, path = self.connection_allowed('HEAD') + if not allowed: self.send_error(403) return - if self.path.startswith(PATHS.MPD): + if path['path'].startswith(PATHS.MPD): try: - file = dict(parse_qsl(urlsplit(self.path).query)).get('file') + file = path['params'].get('file') if file: file_path = os.path.join(self.BASE_PATH, file) else: @@ -266,10 +298,10 @@ def do_HEAD(self): self.end_headers() except IOError: response = ('File Not Found: |{path}| -> |{file_path}|' - .format(path=self.path, file_path=file_path)) + .format(path=path['log_path'], file_path=file_path)) self.send_error(404, response) - elif self.path.startswith(PATHS.REDIRECT): + elif path['path'].startswith(PATHS.REDIRECT): self.send_error(404) else: @@ -277,11 +309,12 @@ def do_HEAD(self): # noinspection PyPep8Naming def do_POST(self): - if not self.connection_allowed('POST'): + allowed, path = self.connection_allowed('POST') + if not allowed: self.send_error(403) return - if self.path.startswith(PATHS.DRM): + if path['path'].startswith(PATHS.DRM): home = xbmcgui.Window(10000) lic_url = home.getProperty('-'.join((ADDON_ID, LICENSE_URL))) diff --git a/resources/lib/youtube_plugin/kodion/utils/methods.py b/resources/lib/youtube_plugin/kodion/utils/methods.py index 450435170..d988f05f5 100644 --- a/resources/lib/youtube_plugin/kodion/utils/methods.py +++ b/resources/lib/youtube_plugin/kodion/utils/methods.py @@ -326,5 +326,6 @@ def wait(timeout=None): return xbmc.Monitor().waitForAbort(timeout) -def redact_ip(url, ip_re=re_compile(r'([?&/])ip([=/])[^?&/]+')): +def redact_ip(url, + ip_re=re_compile(r'([?&/]|%3F|%26|%2F)ip([=/]|%3D|%2F)[^?&/%]+')): return ip_re.sub(r'\g<1>ip\g<2>', url)