diff --git a/astrbot/core/dashboard_assets.py b/astrbot/core/dashboard_assets.py index e94e80631b..8c894e1517 100644 --- a/astrbot/core/dashboard_assets.py +++ b/astrbot/core/dashboard_assets.py @@ -211,7 +211,7 @@ async def _download_package( proxy: str | None = None, progress_callback=None, extract: bool = True, - allow_insecure_ssl_fallback: bool = True, + allow_insecure_ssl_fallback: bool = False, ) -> None: """Download a Dashboard package pinned to one Core version. @@ -223,7 +223,8 @@ async def _download_package( progress_callback: Internal download progress callback. extract: Whether to extract the downloaded package. allow_insecure_ssl_fallback: Whether certificate failures may retry with - TLS verification disabled. + TLS verification disabled. Defaults to ``False`` to avoid silent + man-in-the-middle attacks against the dashboard asset download. Raises: RuntimeError: If neither source provides a valid ZIP package. diff --git a/astrbot/core/platform/sources/misskey/misskey_api.py b/astrbot/core/platform/sources/misskey/misskey_api.py index 3e5eb9a90e..8173a47ec9 100644 --- a/astrbot/core/platform/sources/misskey/misskey_api.py +++ b/astrbot/core/platform/sources/misskey/misskey_api.py @@ -706,24 +706,11 @@ async def upload_and_find_file( import os import tempfile - # SSL 验证下载，失败则重试不验证 SSL - tmp_bytes = None - try: - tmp_bytes = await self._download_with_existing_session( - url, - ssl_verify=True, - ) or await self._download_with_temp_session(url, ssl_verify=True) - except Exception as ssl_error: - logger.debug( - f"[Misskey API] SSL 验证下载失败: {ssl_error}，重试不验证 SSL", - ) - try: - tmp_bytes = await self._download_with_existing_session( - url, - ssl_verify=False, - ) or await self._download_with_temp_session(url, ssl_verify=False) - except Exception: - pass + # 下载文件时强制进行 TLS 校验，避免 MITM 攻击注入恶意内容。 + tmp_bytes = await self._download_with_existing_session( + url, + ssl_verify=True, + ) or await self._download_with_temp_session(url, ssl_verify=True) if tmp_bytes: with tempfile.NamedTemporaryFile(delete=False) as tmpf: diff --git a/astrbot/core/star/updater.py b/astrbot/core/star/updater.py index c82a4546cb..59dcf9178e 100644 --- a/astrbot/core/star/updater.py +++ b/astrbot/core/star/updater.py @@ -20,7 +20,7 @@ from astrbot.core.utils.io import ensure_dir, remove_dir from ..star.star import StarMetadata -from ..zip_updater import _RepoZipUpdater +from ..zip_updater import _RepoZipUpdater, _zip_slip_filter PLUGIN_METADATA_FILENAMES = ("metadata.yaml", "metadata.yml") PLUGIN_METADATA_REQUIRED_FIELDS = ("name", "desc", "version", "author") @@ -459,6 +459,6 @@ def _extract_plugin_archive(self, zip_path: str, target_dir: str) -> None: logger.info(f"Extracting archive: {zip_path}") with zipfile.ZipFile(zip_path, "r") as z: update_dir = self._resolve_archive_root_dir(z.namelist()) - z.extractall(target_dir) + z.extractall(target_dir, filter=_zip_slip_filter) self._finalize_extracted_archive(zip_path, target_dir, update_dir) diff --git a/astrbot/core/utils/io.py b/astrbot/core/utils/io.py index a36bda5e4d..6a6d42a87e 100644 --- a/astrbot/core/utils/io.py +++ b/astrbot/core/utils/io.py @@ -20,6 +20,14 @@ logger = logging.getLogger("astrbot") +# 2 GiB safety cap for streamed HTTP downloads. Content-Length headers above this +# threshold cause the download to abort before any bytes are written to disk. +_DOWNLOAD_MAX_BYTES = 2 * 1024 * 1024 * 1024 + + +class DownloadTooLargeError(RuntimeError): + """Raised when an HTTP download exceeds the configured size cap.""" + def _safe_url_for_log(url: str) -> str: """Return a URL summary that omits query strings and fragments. @@ -117,57 +125,47 @@ async def download_image_by_url( path: str | None = None, ) -> str: """下载图片, 返回 path""" - try: - ssl_context = ssl.create_default_context( - cafile=certifi.where(), - ) # 使用 certifi 提供的 CA 证书 - connector = aiohttp.TCPConnector(ssl=ssl_context) # 使用 certifi 的根证书 - async with aiohttp.ClientSession( - trust_env=True, - connector=connector, - ) as session: - if post: - async with session.post(url, json=post_data) as resp: - if not path: - return save_temp_img(await resp.read()) - with open(path, "wb") as f: - f.write(await resp.read()) - return path - else: - async with session.get(url) as resp: - if not path: - return save_temp_img(await resp.read()) - with open(path, "wb") as f: - f.write(await resp.read()) - return path - except (aiohttp.ClientConnectorSSLError, aiohttp.ClientConnectorCertificateError): - # 关闭SSL验证（仅在证书验证失败时作为fallback） - logger.warning( - f"SSL certificate verification failed for {_safe_url_for_log(url)}. " - "Disabling SSL verification (CERT_NONE) as a fallback. " - "This is insecure and exposes the application to man-in-the-middle attacks. " - "Please investigate and resolve certificate issues." - ) - ssl_context = ssl.create_default_context() - ssl_context.check_hostname = False - ssl_context.verify_mode = ssl.CERT_NONE - async with aiohttp.ClientSession() as session: - if post: - async with session.post(url, json=post_data, ssl=ssl_context) as resp: - if not path: - return save_temp_img(await resp.read()) - with open(path, "wb") as f: - f.write(await resp.read()) - return path - else: - async with session.get(url, ssl=ssl_context) as resp: - if not path: - return save_temp_img(await resp.read()) - with open(path, "wb") as f: - f.write(await resp.read()) - return path - except Exception as e: - raise e + ssl_context = ssl.create_default_context( + cafile=certifi.where(), + ) # 使用 certifi 提供的 CA 证书 + connector = aiohttp.TCPConnector(ssl=ssl_context) # 使用 certifi 的根证书 + async with aiohttp.ClientSession( + trust_env=True, + connector=connector, + ) as session: + if post: + async with session.post(url, json=post_data) as resp: + resp.raise_for_status() + return await _download_image_to_dest(resp, path) + else: + async with session.get(url) as resp: + resp.raise_for_status() + return await _download_image_to_dest(resp, path) + + +async def _download_image_to_dest( + resp: aiohttp.ClientResponse, path: str | None +) -> str: + """Stream download an image with size cap, saving to path or temp file.""" + total_read = 0 + chunks: list[bytes] = [] + + async for chunk in resp.content.iter_chunked(64 * 1024): + if not chunk: + break + total_read += len(chunk) + if total_read > _DOWNLOAD_MAX_BYTES: + raise DownloadTooLargeError( + f"Image download exceeded size cap of {_DOWNLOAD_MAX_BYTES} bytes" + ) + chunks.append(chunk) + + data = b"".join(chunks) + if path: + with open(path, "wb") as f: + f.write(data) + return path + return save_temp_img(data) async def _emit_download_progress(progress_callback, payload: dict) -> None: @@ -217,6 +215,11 @@ async def _download_response_to_file( """ total_size = int(resp.headers.get("content-length", 0)) + if total_size and total_size > _DOWNLOAD_MAX_BYTES: + raise DownloadTooLargeError( + f"Refusing download from {_safe_url_for_log(url)}: advertised size " + f"{total_size} bytes exceeds {_DOWNLOAD_MAX_BYTES} byte cap.", + ) downloaded_size = 0 start_time = time.time() if show_progress: @@ -241,8 +244,13 @@ async def _download_response_to_file( chunk = await resp.content.read(8192) if not chunk: break - file_obj.write(chunk) downloaded_size += len(chunk) + if downloaded_size > _DOWNLOAD_MAX_BYTES: + raise DownloadTooLargeError( + f"Aborting download from {_safe_url_for_log(url)}: response " + f"exceeded {_DOWNLOAD_MAX_BYTES} byte cap.", + ) + file_obj.write(chunk) elapsed_time = time.time() - start_time if time.time() - start_time > 0 else 1 speed = downloaded_size / 1024 / elapsed_time # KB/s percent = downloaded_size / total_size if total_size > 0 else 0 @@ -278,7 +286,7 @@ async def download_file( path: str, show_progress: bool = False, progress_callback=None, - allow_insecure_ssl_fallback: bool = True, + allow_insecure_ssl_fallback: bool = False, ) -> None: """Download a remote file to a local path. @@ -288,7 +296,9 @@ async def download_file( show_progress: Whether to print progress to stdout. progress_callback: Optional callback for progress payloads. allow_insecure_ssl_fallback: Whether certificate failures may retry with - TLS certificate verification disabled. + TLS certificate verification disabled. Defaults to ``False`` because + silently disabling certificate verification enables man-in-the-middle + attacks against users and remote plugin/dashboard downloads. Returns: None. @@ -342,6 +352,14 @@ async def download_file( progress_callback, show_downloading_label=False, ) + except DownloadTooLargeError: + # Remove the partial file written before the size cap tripped so a + # truncated/empty artifact is not left on disk. + try: + os.remove(path) + except FileNotFoundError: + pass + raise if show_progress: print() diff --git a/astrbot/core/zip_updater.py b/astrbot/core/zip_updater.py index 9ba11998ed..bb16c3ac69 100644 --- a/astrbot/core/zip_updater.py +++ b/astrbot/core/zip_updater.py @@ -17,6 +17,28 @@ __all__ = ["ReleaseInfo"] +def _zip_slip_filter(member: zipfile.ZipInfo, path: str | bytes) -> zipfile.ZipInfo: + """Reject zip members that would escape the destination directory (CWE-22). + + Mirrors the behaviour of ``zipfile.ZipFile.extractall(filter="data")`` but is + written explicitly so the safety check stays consistent across Python 3.12 + and newer minor releases and is easy to unit test. + """ + + base = os.path.abspath(path) + target = os.path.abspath(os.path.join(base, member.filename)) + try: + common = os.path.commonpath([base, target]) + except ValueError: + # Raised when paths live on different drives on Windows. + common = "" + if common != base: + raise ValueError( + f"Refusing zip member that escapes destination: {member.filename!r}" + ) + return member + + class ReleaseInfo: """Describe a repository release exposed by an updater. @@ -315,7 +337,7 @@ def _extract_archive(self, zip_path: str, target_dir: str) -> None: ensure_dir(target_dir) with zipfile.ZipFile(zip_path, "r") as z: update_dir = self._resolve_archive_root_dir(z.namelist()) - z.extractall(target_dir) + z.extractall(target_dir, filter=_zip_slip_filter) logger.debug(f"Finished extracting archive: {zip_path}") self._finalize_extracted_archive(zip_path, target_dir, update_dir) diff --git a/tests/test_updater_socks.py b/tests/test_updater_socks.py index 8dd3bf6daa..70f7772fb2 100644 --- a/tests/test_updater_socks.py +++ b/tests/test_updater_socks.py @@ -192,7 +192,7 @@ def read(self, name: str) -> bytes: ) return b"" - def extractall(self, target_dir: str) -> None: # noqa: ARG002 + def extractall(self, target_dir: str, filter=None) -> None: # noqa: ARG002 return None