From e5ae88cf0dca1c98f8c07084db108bc9f0aae18b Mon Sep 17 00:00:00 2001 From: wjc <139726196+wjc2821296948@users.noreply.github.com> Date: Fri, 7 Aug 2026 04:20:13 +0800 Subject: [PATCH 1/5] fix(security): remove silent TLS verification fallback in download helpers io.download_file() and io.download_image_by_url() silently retried HTTPS requests with ssl.CERT_NONE when certificate verification failed. A network attacker could intercept these connections and serve malicious content (plugin/dashboard assets, chat media) without triggering any user-visible warning. Change the default of download_file() and dashboard_assets._download_package() to allow_insecure_ssl_fallback=False. Drop the silent CERT_NONE retry in download_image_by_url() entirely so SSL errors propagate to the caller. Existing callers that intentionally opt into the fallback keep the keyword argument and behavior. Co-authored-by: cgsdn --- astrbot/core/dashboard_assets.py | 5 +- astrbot/core/utils/io.py | 79 +++++++++++--------------------- 2 files changed, 29 insertions(+), 55 deletions(-) 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/utils/io.py b/astrbot/core/utils/io.py index a36bda5e4d..749205c883 100644 --- a/astrbot/core/utils/io.py +++ b/astrbot/core/utils/io.py @@ -117,57 +117,28 @@ 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: + 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 async def _emit_download_progress(progress_callback, payload: dict) -> None: @@ -278,7 +249,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 +259,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. From 2876c406e27e6ed811054a3c469a49bae697b46a Mon Sep 17 00:00:00 2001 From: wjc <139726196+wjc2821296948@users.noreply.github.com> Date: Fri, 7 Aug 2026 13:37:44 +0800 Subject: [PATCH 2/5] fix(security): stop silently disabling TLS for Misskey URL uploads upload_and_find_file() caught SSL verification errors and retried the download with ssl_verify=False, allowing a network attacker to serve arbitrary bytes that would then be uploaded to the Misskey instance as if they came from the legitimate URL. Drop the CERT_NONE retry entirely so certificate errors propagate to the caller instead of being masked. Co-authored-by: cgsdn --- .../platform/sources/misskey/misskey_api.py | 23 ++++--------------- 1 file changed, 5 insertions(+), 18 deletions(-) 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: From f3c32166b9c29f1b1476b7860d0f0d626ff7e806 Mon Sep 17 00:00:00 2001 From: wjc <139726196+wjc2821296948@users.noreply.github.com> Date: Fri, 7 Aug 2026 13:53:03 +0800 Subject: [PATCH 3/5] fix(security): reject zip-slip during archive extraction _RepoZipUpdater._extract_archive() and _PluginUpdater._extract_plugin_archive() called zipfile.ZipFile.extractall(target_dir) without validating archive member paths. A crafted plugin or update archive could include members like ../..//evil.py, which zipfile writes outside target_dir during extractall and only moves inside the finalize step. The later commonpath() check rejects the move but the file is already on disk. Pass a path-traversal filter to extractall() that raises ValueError before any member whose destination escapes the target directory is written. Update the existing test fake ZipFile to accept the new keyword argument. Co-authored-by: cgsdn --- astrbot/core/star/updater.py | 4 ++-- astrbot/core/zip_updater.py | 24 +++++++++++++++++++++++- tests/test_updater_socks.py | 2 +- 3 files changed, 26 insertions(+), 4 deletions(-) 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/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 From de43eb2a0d9580cd46b262cdf0449e6db155af05 Mon Sep 17 00:00:00 2001 From: wjc <139726196+wjc2821296948@users.noreply.github.com> Date: Fri, 7 Aug 2026 14:00:51 +0800 Subject: [PATCH 4/5] fix(security): cap streamed download size to prevent disk-fill DoS download_file() streamed the response body to disk in 8 KiB chunks and only used the Content-Length header to compute progress, never to bound the download. A malicious or misconfigured remote could advertise a huge Content-Length or stream unlimited bytes, exhausting free disk space before any other validation runs. Reject responses whose advertised Content-Length exceeds a 2 GiB cap before writing any bytes, abort the stream as soon as the running total crosses that cap, and remove the partial file the size guard tripped on so a truncated artifact is not left behind. Co-authored-by: cgsdn --- astrbot/core/utils/io.py | 28 +++++++++++++++++++++++++++- 1 file changed, 27 insertions(+), 1 deletion(-) diff --git a/astrbot/core/utils/io.py b/astrbot/core/utils/io.py index 749205c883..e76ea5dd8f 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. @@ -188,6 +196,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: @@ -212,8 +225,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 @@ -315,6 +333,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() From ebca0abf698fb9581f9049794bb7b40050e1b475 Mon Sep 17 00:00:00 2001 From: wjc <139726196+wjc2821296948@users.noreply.github.com> Date: Sat, 8 Aug 2026 00:38:54 +0800 Subject: [PATCH 5/5] fix(security): apply download size cap to download_image_by_url DownloadImageByUrl previously read the full response body without any size check, bypassing the _DOWNLOAD_MAX_BYTES protection added for streamed file downloads. Stream the response in 64 KiB chunks, enforce the same cap, and raise DownloadTooLargeError when exceeded. Co-authored-by: cgsdn --- astrbot/core/utils/io.py | 39 +++++++++++++++++++++++++++++---------- 1 file changed, 29 insertions(+), 10 deletions(-) diff --git a/astrbot/core/utils/io.py b/astrbot/core/utils/io.py index e76ea5dd8f..6a6d42a87e 100644 --- a/astrbot/core/utils/io.py +++ b/astrbot/core/utils/io.py @@ -135,18 +135,37 @@ async def download_image_by_url( ) 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 + resp.raise_for_status() + return await _download_image_to_dest(resp, 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 + 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: